After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 704999 - glib/convert.test crashing due to lack of iconv cache
glib/convert.test crashing due to lack of iconv cache
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2013-07-27 17:50 UTC by Colin Walters
Modified: 2013-07-28 22:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
configure: Reintroduce check for glibc (802 bytes, patch)
2013-07-27 23:22 UTC, Colin Walters
none Details | Review
Drop iconv caching code (8.91 KB, patch)
2013-07-28 14:20 UTC, Colin Walters
accepted-commit_now Details | Review

Description Colin Walters 2013-07-27 17:50:34 UTC
{ "__CURSOR" : "s=05a63bb91d504009b5257fe37a2f88eb;i=57b;b=3491d960457f488fb61c272ee54f58bf;m=c617d3;t=4e280342e720a;x=ced9b98d05192d8a", "__REALTIME_TIMESTAMP" : "1374940165992970", "__MONOTONIC_TIMESTAMP" : "12982227", "_BOOT_ID" : "3491d960457f488fb61c272ee54f58bf", "_TRANSPORT" : "stdout", "PRIORITY" : "6", "_PID" : "423", "_UID" : "1000", "_GID" : "1000", "_SYSTEMD_CGROUP" : "/user.slice/1000.user/c1.session", "_SYSTEMD_SESSION" : "c1", "_SYSTEMD_OWNER_UID" : "1000", "_MACHINE_ID" : "45bb3b96146aa94f299b9eb43646eb35", "_HOSTNAME" : "qemux86-64", "SYSLOG_IDENTIFIER" : "gnome-session", "_COMM" : "gnome-session", "_EXE" : "/usr/bin/gnome-session", "_CMDLINE" : "gnome-session", "MESSAGE" : "Running test: glib/convert.test" }
{ "__CURSOR" : "s=67656a44781a4d8fb81e55a3963d86ab;i=c2e;b=3491d960457f488fb61c272ee54f58bf;m=1189d16;t=4e2803480f74c;x=c514ad1101da323e", "__REALTIME_TIMESTAMP" : "1374940171401036", "__MONOTONIC_TIMESTAMP" : "18390294", "_BOOT_ID" : "3491d960457f488fb61c272ee54f58bf", "PRIORITY" : "6", "_TRANSPORT" : "stdout", "_MACHINE_ID" : "45bb3b96146aa94f299b9eb43646eb35", "_HOSTNAME" : "qemux86-64", "_GID" : "1000", "_UID" : "1000", "SYSLOG_IDENTIFIER" : "glib/convert.test", "MESSAGE" : "/conversion/no-conv: OK", "_PID" : "2126" }
{ "__CURSOR" : "s=67656a44781a4d8fb81e55a3963d86ab;i=c2f;b=3491d960457f488fb61c272ee54f58bf;m=1189ddf;t=4e2803480f815;x=234ed8a30dbc9373", "__REALTIME_TIMESTAMP" : "1374940171401237", "__MONOTONIC_TIMESTAMP" : "18390495", "_BOOT_ID" : "3491d960457f488fb61c272ee54f58bf", "PRIORITY" : "6", "_TRANSPORT" : "stdout", "_MACHINE_ID" : "45bb3b96146aa94f299b9eb43646eb35", "_HOSTNAME" : "qemux86-64", "_GID" : "1000", "_UID" : "1000", "SYSLOG_IDENTIFIER" : "glib/convert.test", "_PID" : "2126", "MESSAGE" : "/conversion/iconv-state: OK" }
{ "__CURSOR" : "s=67656a44781a4d8fb81e55a3963d86ab;i=c30;b=3491d960457f488fb61c272ee54f58bf;m=1189ea1;t=4e2803480f8d7;x=a01a15283897abe7", "__REALTIME_TIMESTAMP" : "1374940171401431", "__MONOTONIC_TIMESTAMP" : "18390689", "_BOOT_ID" : "3491d960457f488fb61c272ee54f58bf", "PRIORITY" : "6", "_TRANSPORT" : "stdout", "_MACHINE_ID" : "45bb3b96146aa94f299b9eb43646eb35", "_HOSTNAME" : "qemux86-64", "_GID" : "1000", "_UID" : "1000", "SYSLOG_IDENTIFIER" : "glib/convert.test", "_PID" : "2126", "MESSAGE" : "/conversion/illegal-sequence: OK" }
{ "__CURSOR" : "s=67656a44781a4d8fb81e55a3963d86ab;i=c31;b=3491d960457f488fb61c272ee54f58bf;m=1189f72;t=4e2803480f9a9;x=3378be9c329475f3", "__REALTIME_TIMESTAMP" : "1374940171401641", "__MONOTONIC_TIMESTAMP" : "18390898", "_BOOT_ID" : "3491d960457f488fb61c272ee54f58bf", "PRIORITY" : "6", "_TRANSPORT" : "stdout", "_MACHINE_ID" : "45bb3b96146aa94f299b9eb43646eb35", "_HOSTNAME" : "qemux86-64", "_GID" : "1000", "_UID" : "1000", "SYSLOG_IDENTIFIER" : "glib/convert.test", "_PID" : "2126", "MESSAGE" : "/conversion/byte-order: **" }
{ "__CURSOR" : "s=67656a44781a4d8fb81e55a3963d86ab;i=c32;b=3491d960457f488fb61c272ee54f58bf;m=118a04d;t=4e2803480fa84;x=658b29cd266e9a52", "__REALTIME_TIMESTAMP" : "1374940171401860", "__MONOTONIC_TIMESTAMP" : "18391117", "_BOOT_ID" : "3491d960457f488fb61c272ee54f58bf", "PRIORITY" : "6", "_TRANSPORT" : "stdout", "_MACHINE_ID" : "45bb3b96146aa94f299b9eb43646eb35", "_HOSTNAME" : "qemux86-64", "_GID" : "1000", "_UID" : "1000", "SYSLOG_IDENTIFIER" : "glib/convert.test", "_PID" : "2126", "MESSAGE" : "ERROR:../../../glib/tests/convert.c:133:test_byte_order: assertion failed: (bytes_written == 2)" }
Comment 1 Colin Walters 2013-07-27 22:59:48 UTC
Ok, this test started crashing due to bug 684123.  The configure check for the iconv cache was using ac_cv_glibc_2_1 which is no longer set.

This test should work without the iconv cache flag though.  And we need to change the configure check to detect iconv-cacheability in a better way.
Comment 2 Colin Walters 2013-07-27 23:18:31 UTC
A brief bit of code archaeology here leads me to commit:

https://git.gnome.org/browse/glib/commit/?id=717f3d4abbe99467f07b05462d7651b08e69dd20

Which in turn leads to:

https://bugzilla.redhat.com/show_bug.cgi?id=165368

So basically we can't use the iconv cache with glibc.  I'm kind of tempted to just turn it off entirely for all platforms...it sounds like it was first introduced for some ancient version of Solaris.  But do they still have that issue?
Comment 3 Colin Walters 2013-07-27 23:22:39 UTC
Created attachment 250280 [details] [review]
configure: Reintroduce check for glibc

Because we need it to avoid using the iconv cache, which
causes errors.

But don't use the glibc check to define _GNU_SOURCE, we still
use AC_USE_SYSTEM_EXTENSIONS for that.
Comment 4 Colin Walters 2013-07-27 23:23:34 UTC
Not going to poke the ancient dragons here, let's just keep doing what we were doing before with respect to iconv caching.
Comment 5 bugdal 2013-07-27 23:52:22 UTC
From what I can tell, including Drepper's comment on the above-linked bug report, the concept of the iconv cache is just wrong. POSIX specifies that a NULL input pointer resets the "shift state" for "state-dependent encodings"; however, it's not clear to me that UTF-16 should be treated as an encoding with "shift states" for this purpose. That language is obviously intended to deal with things like ISO-2022, where sequences to change the shift state are valid at any point in the input. Treating the beginning of input specially is a quirk of UTF-16 BOM monster, and it's not really compatible with the iconv API at all, so short of an interpretation from the committee on how iconv is supposed to behave, I don't think we can say either behavior is right or wrong.

Personally, I vote for just removing the iconv cache code altogether and filing upstream bug reports for any operating system where iconv_open is intolerably slow.

If that's not acceptable, the reasonable alternative would be either:

1. Adding a check in configure that runs a test program to determine the behavior of iconv for UTF-16, and turns off cache if the BOM is not re-processed after the "reset state" request. Obviously this test should default to the safe option of turning the cache off when a test program cannot be run (cross compiling).

2. Adding a run-time check in the cache code for UTF-16 behavior, and refusing to cache descriptors for UTF-16 if they don't behave as desired.
Comment 6 Colin Walters 2013-07-28 14:20:40 UTC
Created attachment 250302 [details] [review]
Drop iconv caching code

This was introduced for Solaris performance theoretically;
we have never been able to use it on Linux/glibc because
the UTF-16 BOM state isn't reset.

We have no data about Solaris performance; were some to
still exist, we could reintroduce the code with an explicit
check for Solaris, not a check for glibc.
Comment 7 Jasper St. Pierre (not reading bugmail) 2013-07-28 19:16:56 UTC
Review of attachment 250302 [details] [review]:

This looks OK.
Comment 8 Colin Walters 2013-07-28 20:03:11 UTC
(In reply to comment #7)
> Review of attachment 250302 [details] [review]:
> 
> This looks OK.

Ok, though I'd like to know what Matthias thinks, since he wrote all the code here originally.