GNOME Bugzilla – Bug 646435
GTimeZone doesn't seem to be thread-safe
Last modified: 2011-04-14 13:57:04 UTC
emilio@marte:/tmp$ gcc `pkg-config --cflags --libs glib-2.0 gthread-2.0 gobject-2.0` -o testcase testcase.c emilio@marte:/tmp$ gdb ./testcase GNU gdb (GDB) 7.2-debian Copyright (C) 2010 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-linux-gnu". For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>... Reading symbols from /tmp/testcase...(no debugging symbols found)...done. (gdb) r Starting program: /tmp/testcase [Thread debugging using libthread_db enabled] [New Thread 0x7ffff6cd6700 (LWP 28116)] [New Thread 0x7ffff64d5700 (LWP 28117)] [New Thread 0x7ffff5cd4700 (LWP 28118)] *** glibc detected *** /tmp/testcase: free(): invalid pointer: 0x000000000060dc60 *** ======= Backtrace: ========= /lib/libc.so.6(+0x71ab6)[0x7ffff6f83ab6] /lib/libc.so.6(cfree+0x6c)[0x7ffff6f8882c] /lib/libglib-2.0.so.0(g_time_zone_unref+0xb0)[0x7ffff7500b50] /lib/libglib-2.0.so.0(g_date_time_unref+0x51)[0x7ffff74bef31] /tmp/testcase[0x400997] /lib/libglib-2.0.so.0(+0x6ccf6)[0x7ffff74fccf6] /lib/libpthread.so.0(+0x68ba)[0x7ffff727a8ba] /lib/libc.so.6(clone+0x6d)[0x7ffff6fe13cd] ======= Memory map: ======== 00400000-00401000 r-xp 00000000 fe:01 13053177 /tmp/testcase 00600000-00601000 rw-p 00000000 fe:01 13053177 /tmp/testcase 00601000-00622000 rw-p 00000000 00:00 0 [heap] 7ffff0000000-7ffff0021000 rw-p 00000000 00:00 0 7ffff0021000-7ffff4000000 ---p 00000000 00:00 0 7ffff52be000-7ffff52d3000 r-xp 00000000 fe:01 6922490 /lib/libgcc_s.so.1 7ffff52d3000-7ffff54d3000 ---p 00015000 fe:01 6922490 /lib/libgcc_s.so.1 7ffff54d3000-7ffff54d4000 rw-p 00015000 fe:01 6922490 /lib/libgcc_s.so.1 7ffff54d4000-7ffff54d5000 ---p 00000000 00:00 0 7ffff54d5000-7ffff5cd5000 rw-p 00000000 00:00 0 7ffff5cd5000-7ffff5cd6000 ---p 00000000 00:00 0 7ffff5cd6000-7ffff64d6000 rw-p 00000000 00:00 0 7ffff64d6000-7ffff64d7000 ---p 00000000 00:00 0 7ffff64d7000-7ffff6cd7000 rw-p 00000000 00:00 0 7ffff6cd7000-7ffff6d12000 r-xp 00000000 fe:01 6922252 /lib/libpcre.so.3.12.1 7ffff6d12000-7ffff6f11000 ---p 0003b000 fe:01 6922252 /lib/libpcre.so.3.12.1 7ffff6f11000-7ffff6f12000 rw-p 0003a000 fe:01 6922252 /lib/libpcre.so.3.12.1 7ffff6f12000-7ffff706a000 r-xp 00000000 fe:01 10862827 /lib/libc-2.11.2.so 7ffff706a000-7ffff726a000 ---p 00158000 fe:01 10862827 /lib/libc-2.11.2.so 7ffff726a000-7ffff726e000 r--p 00158000 fe:01 10862827 /lib/libc-2.11.2.so 7ffff726e000-7ffff726f000 rw-p 0015c000 fe:01 10862827 /lib/libc-2.11.2.so 7ffff726f000-7ffff7274000 rw-p 00000000 00:00 0 7ffff7274000-7ffff728b000 r-xp 00000000 fe:01 10862829 /lib/libpthread-2.11.2.so 7ffff728b000-7ffff748a000 ---p 00017000 fe:01 10862829 /lib/libpthread-2.11.2.so 7ffff748a000-7ffff748b000 r--p 00016000 fe:01 10862829 /lib/libpthread-2.11.2.so 7ffff748b000-7ffff748c000 rw-p 00017000 fe:01 10862829 /lib/libpthread-2.11.2.so 7ffff748c000-7ffff7490000 rw-p 00000000 00:00 0 7ffff7490000-7ffff7580000 r-xp 00000000 fe:01 6922352 /lib/libglib-2.0.so.0.2800.4 7ffff7580000-7ffff777f000 ---p 000f0000 fe:01 6922352 /lib/libglib-2.0.so.0.2800.4 7ffff777f000-7ffff7780000 rw-p 000ef000 fe:01 6922352 /lib/libglib-2.0.so.0.2800.4 7ffff7780000-7ffff7781000 rw-p 00000000 00:00 0 7ffff7781000-7ffff7788000 r-xp 00000000 fe:01 10862831 /lib/librt-2.11.2.so 7ffff7788000-7ffff7987000 ---p 00007000 fe:01 10862831 /lib/librt-2.11.2.so 7ffff7987000-7ffff7988000 r--p 00006000 fe:01 10862831 /lib/librt-2.11.2.so 7ffff7988000-7ffff7989000 rw-p 00007000 fe:01 10862831 /lib/librt-2.11.2.so 7ffff7989000-7ffff798d000 r-xp 00000000 fe:01 2253336 /usr/lib/libgthread-2.0.so.0.2800.4 7ffff798d000-7ffff7b8c000 ---p 00004000 fe:01 2253336 /usr/lib/libgthread-2.0.so.0.2800.4 7ffff7b8c000-7ffff7b8d000 rw-p 00003000 fe:01 2253336 /usr/lib/libgthread-2.0.so.0.2800.4 7ffff7b8d000-7ffff7bdd000 r-xp 00000000 fe:01 2253370 /usr/lib/libgobject-2.0.so.0.2800.4 7ffff7bdd000-7ffff7ddc000 ---p 00050000 fe:01 2253370 /usr/lib/libgobject-2.0.so.0.2800.4 7ffff7ddc000-7ffff7dde000 rw-p 0004f000 fe:01 2253370 /usr/lib/libgobject-2.0.so.0.2800.4 7ffff7dde000-7ffff7ddf000 rw-p 00000000 00:00 0 7ffff7ddf000-7ffff7dfd000 r-xp 00000000 fe:01 10862848 /lib/ld-2.11.2.so 7ffff7fd7000-7ffff7fdc000 rw-p 00000000 00:00 0 7ffff7ff8000-7ffff7ff9000 r--p 00000000 fe:01 2261723 /usr/share/zoneinfo/UTC 7ffff7ff9000-7ffff7ffb000 rw-p 00000000 00:00 0 7ffff7ffb000-7ffff7ffc000 r-xp 00000000 00:00 0 [vdso] 7ffff7ffc000-7ffff7ffd000 r--p 0001d000 fe:01 10862848 /lib/ld-2.11.2.so 7ffff7ffd000-7ffff7ffe000 rw-p 0001e000 fe:01 10862848 /lib/ld-2.11.2.so 7ffff7ffe000-7ffff7fff000 rw-p 00000000 00:00 0 7ffffffde000-7ffffffff000 rw-p 00000000 00:00 0 [stack] ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall] Program received signal SIGABRT, Aborted.
+ Trace 226526
Thread 140737317259008 (LWP 28118)
emilio@marte:/tmp$ G_DEBUG=gc-friendly G_SLICE=debug-blocks valgrind --tool=memcheck ./testcase ==28228== Memcheck, a memory error detector ==28228== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al. ==28228== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info ==28228== Command: ./testcase ==28228== ** GLib:ERROR:/tmp/buildd/glib2.0-2.28.4/./glib/gtimezone.c:195:g_time_zone_ref: assertion failed: (tz->ref_count > 0) ==28228== ==28228== HEAP SUMMARY: ==28228== in use at exit: 96,509 bytes in 398 blocks ==28228== total heap usage: 25,388 allocs, 24,990 frees, 530,455 bytes allocated ==28228== ==28228== LEAK SUMMARY: ==28228== definitely lost: 0 bytes in 0 blocks ==28228== indirectly lost: 0 bytes in 0 blocks ==28228== possibly lost: 15,743 bytes in 208 blocks ==28228== still reachable: 80,766 bytes in 190 blocks ==28228== suppressed: 0 bytes in 0 blocks ==28228== Rerun with --leak-check=full to see details of leaked memory ==28228== ==28228== For counts of detected and suppressed errors, rerun with: -v ==28228== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4) Killed
Created attachment 184863 [details] [review] testcase
Created attachment 184864 [details] [review] minimal testcase This testcase doesn't involve GDateTime but just GTimeZone.
Looks to me like the unref is not really threadsafe: if (g_atomic_int_dec_and_test (&tz->ref_count)) { G_LOCK(time_zones); g_hash_table_remove (time_zones, tz->name); G_UNLOCK(time_zones); Because some other thread can still get the tz out of the hash table between the dec_and_test and the lock
moving the lock/unlock outside the atomic dec_and_test doesn't make the test crash for me.
Created attachment 184880 [details] [review] Lock cache while freeing GTimeZone Same for me, and it's logical since you don't want to grab a GTimeZone from cache that has ref == 0. This would get messy afterward since you could endup with two or more concurrent unref entering the freeing part.
Review of attachment 184880 [details] [review]: ::: glib/gtimezone.c @@ +149,3 @@ g_assert (tz->ref_count > 0); + G_LOCK(time_zones); I'm not crazy about how we acquire a lock on every unref...
Created attachment 185910 [details] [review] Safely remove timezone from cache on unref This is a new patch that does not lock everytime you unref. Take this has one solution, I know Ryan is working on another approach, which might be simplier.
commit 8b03077a44092ce2b510ef3246da063cacc8d951 Author: Ryan Lortie <desrt@desrt.ca> Date: Thu Apr 14 09:54:17 2011 -0400 GTimeZone: fix non-threadsafe refcounting In the previous code, if the timezone was pulled out of the cache again just as the last reference was being dropped, the cache code will increase its refcount and return it while the unref code was freeing it. Protect against that. Closes #646435. I'm not satisfied that this is the best possible solution and I want to think about it some more. In the meantime, though, we should probably fix this crash.