Bring forward Mark Wielaard comments from sourceware to upstream:


This is most likely caused by mixing different allocation strategies with the
wrong deallocation strategy. In previous gtk+ releases everything was allocated
through malloc and freed through g_free (which maps to free). But with
introduction of the gslice allocator in GLib 2.10 you have to free or unref
things properly (you cannot just call gfree on everything because not all [small
objects] are allocated through malloc anymore, you have to use g_object_unref
where appropriate).

The above theory is supported by the fact that running with
G_SLICE=always-malloc makes the problem disappear.

For more info see:
http://www.gnome.org/start/2.14/notes/en/rndevelopers.html
http://developer.gnome.org/doc/API/2.0/glib/glib-Memory-Slices.html
And the following thread on the problem in various applications:
http://lists.debian.org/debian-devel/2006/03/msg00941.html

FWIW I see the failure in processPendingStruct() in gtk_java.c

It fails with an invalid pointer (among other things) on the unconditional g_free.

Different things in Glib need to be freed in different ways. As of java-gnome 2.12.0, libgtk-java and friends had a massive memory management upgrade to handle the different cases.

*Personally* I don't much care for way those internalls are handled, but that's why I'm working on a re-engineering of the bindings layer. Irrespective of that, the memory management part certainly seems, in all fairness, to be reasonably comprehesive. The GObject case and Boxed cases certainly seem to work fine.

Assuming the analysis in #2, my guess would be that some Type is being misregistered as a Struct instead of as a {something else}. What's presently called processPendingBLAH() is what gets called when a struct is told its ready for freeing/unref'ing. My speculations are:

* could be that we don't actually own the return value of whatever is being free'd and so trying to free it is causing a mess;

* could be that a code path is causing this sturcture to be free'd twice; or

* could be that it's not a Struct.

GdkRegion, huh? Note that it's after a _copy() call. That sort of semantic is more what I'd expect to see from a GBoxed, not just a raw struct.

And sure enough, looking at the defs file from pygtk, it's a boxed alright:

(define-boxed Region
  (in-module "Gdk")
  (c-name "GdkRegion")
  (gtype-id "PYGDK_TYPE_REGION")
)

... though I'm surprised not to see the copy and free functions explicitly mentioned there - absent from GdkRectangle as well. That is either

a) a bug for them (and for us in the future tense); or it may be that

b) bindings don't actually have to worry about it at all as the Glib type system is given the copy and release functions at registration time [I'm still ascertaining that, though it seems likely, and if so, dramatically simplifies things].

Anyway, my suggestion would be to try changing Region from a Struct into a Boxed and see if that eliminates the bug. That would be s/{Mem,}Struct/Boxed/g in

src/java/org/gnu/gdk/Region.c
src/jni/org_gnu_gdk_Region.c

AfC

As an update, we have found the area of the code that triggers the pointer weirdness in Java-GNOME:

    Window drawingArea = this.getWindow(TextWindowType.LEFT);

    // draw the background for the margin
    if (this.myContext == null)
      this.myContext = new GC((Drawable) drawingArea);

    myContext.setRGBForeground(marginColor);
    drawingArea.drawRectangle(this.myContext, true, 0, 0,
                              drawingArea.getWidth(), drawingArea.getHeight());

    // get the y coordinates for the top and bottom of the window
    int minY = drawingArea.getClipRegion().getClipbox().getY();
    int maxY = minY + drawingArea.getClipRegion().getClipbox().getHeight();

Specifically, if we comment out the two getClipRegion() lines of code above, the crash never happens. I did try and write a finalize() for Region that did an explicit: gdk_region_destroy() and then called super.finalize(). No luck there.

Well at least we have a small testcase to work with now ;)

As an aside, gtk_java.c is being slimmed down by the work in bug #350092; perhaps that will help.

AfC

Looking at the valgrind trace, in particular the second part where it reports that the object still has an outstanding reference.  Presumably the later freeing of that second reference is what triggers the double-free panic.

Looking at that outstanding reference, I can see two possible origins:

- it is from the G_SLICE pool's internal structures

The object was directly free()d when it should have been returned to that pool and then allowed the poll to clean up internal structures.

- it is from a second structure which still has a reference

Tends to suggest some sort of reference count botch.  Could a duplicate reference to the object have been created behind JG's back?  Could a duplicate reference have been created with the assumption that the old reference is automatially dropped on the floor?


Of these I tend to suspect the second since I'd have expected the first senario to exhibit itself in other ways, bad pointer passed to free for instance.

Just an update - I've tried Andrew's idea in comment #3 of changing the Structs to Boxeds in those files, no dice.

Similarly I applied the patch from #350092 also didn't run into any luck. Even both at the same time, unfortunately.

I also tried mantaining a global reference to the Region object through Java during the running of Frysk, and that didn't help either.

If someone could boil the code fragment that Phil describes in comment #4 down to an actual executable program example, it would really help. That snippet has a fair bit of context. If you can get isolate a small test case and attach it to this bug, then we might be able to have a go at this.

[None of us have a Fedora + Frysk system around. It will be instructive to see whether or not someone else can replicate the problem. I don't believe this is an OS specific problem, but it would not be a good idea to go bashing away at things without being able to exhibit the problem first]

AfC

(In reply to comment #8)

Obviously, and mcvet attempted this.  That code snipet in relative isolation doesn't crash :-(

I'm also suspecting that valgrind is reporting something different to the actual crasher - a mostly harmless querk in the way refreshes work.

> If someone could boil the code fragment that Phil describes in comment #4 down
> to an actual executable program example, it would really help. That snippet has
> a fair bit of context. If you can get isolate a small test case and attach it
> to this bug, then we might be able to have a go at this.
> 
> [None of us have a Fedora + Frysk system around. It will be instructive to see
> whether or not someone else can replicate the problem. I don't believe this is
> an OS specific problem, but it would not be a good idea to go bashing away at
> things without being able to exhibit the problem first]
> 
> AfC
> 

Created attachment 75379 [details]
GLIBC crash during GC due to refresh and regions

The result of three days brain storming and GTK code study by MCVET, CAGNEY, and PMULDOON.

The refresh and graphical context both appear to be required.  Without them the bad free doesn't occure.  Not a simple pointer munge and not a simple GC failure.

The refresh is especially important - the bad free detected by valgrind.  GTK internally made a copy of the pointer adding it to its own temporary region which is then, during the refresh, released.

FC 6 Released

Created attachment 75384 [details]
GLIBC crash during GC due to refresh and regions

Test case reproduces crash on my system (hooray!). Well done Phil and Andrew.

AfC

Observations:

1. Did you know the crash happens [in the test case] at app termination, not during the gc() or the sleep()?

2. The two calls to getClipRegion() return different objects, which surprised me. I had been under the impression that there was some logic to lookup and return the same object; specifically, glib-java/src/jni/glib_java.c contains getGObjectHandle() which seems to try and use an existing object. 

3. (2) was evident when I split the drawingArea.getClipRegion().getClipbox().getHeight() chains into individual statements.

4. By doing (3) ie commenting out the orginal chained getters] the crash does *NOT* occur. Do the chained getters (again, after the individual statements) and the crash returns.

AfC

Running test, under valgrind, on FC 5 does not appear to get a double free barf.  (perhaps it happens so rarely for me to not yet see it)

glibc barf for testcase on FC-6 2006-10-26

*** glibc detected *** ./frysk-gtk/tests/frysk3116/RegionAndGCFailure: free(): invalid pointer: 0x09eeb420 ***
======= Backtrace: =========
/lib/libc.so.6[0x48bebefd]
/lib/libc.so.6(cfree+0x90)[0x48bef550]
/lib/libglib-2.0.so.0(g_free+0x31)[0x48d785f1]
/usr/lib/libgtkjni-2.8.so(processPendingStruct+0x98)[0x3af328]
/usr/lib/libglibjni-0.2.so(jg_process_atexit+0x52)[0x9ef592]
/lib/libc.so.6(exit+0xe9)[0x48bb1939]
/usr/lib/libgcj.so.7rh[0x4ae2cc97]
======= Memory map: ========
00001000-000b6000 rwxp 00001000 00:00 0
00178000-00180000 r-xp 00000000 fd:00 46465069   /lib/libnss_nis-2.5.so
00180000-00181000 r-xp 00007000 fd:00 46465069   /lib/libnss_nis-2.5.so
00181000-00182000 rwxp 00008000 fd:00 46465069   /lib/libnss_nis-2.5.so
00283000-0028c000 r-xp 00000000 fd:00 46465065   /lib/libnss_files-2.5.so
0028c000-0028d000 r-xp 00008000 fd:00 46465065   /lib/libnss_files-2.5.so
0028d000-0028e000 rwxp 00009000 fd:00 46465065   /lib/libnss_files-2.5.so
00344000-00416000 r-xp 00000000 fd:00 36897065   /usr/lib/libgtkjni-2.8.so
00416000-00419000 rwxp 000d2000 fd:00 36897065   /usr/lib/libgtkjni-2.8.so
00485000-00496000 r-xp 00000000 fd:00 25723402   /usr/lib/gtk-2.0/2.10.0/engines/libclearlooks.so
00496000-00497000 rwxp 00011000 fd:00 25723402   /usr/lib/gtk-2.0/2.10.0/engines/libclearlooks.so
006f8000-006fa000 r-xp 00000000 fd:00 25692418   /usr/lib/pango/1.5.0/modules/pango-basic-fc.so
006fa000-006fb000 rwxp 00001000 fd:00 25692418   /usr/lib/pango/1.5.0/modules/pango-basic-fc.so
00972000-00973000 r-xp 00000000 fd:00 25527233   /usr/lib/gconv/ISO8859-1.so
00973000-00975000 rwxp 00000000 fd:00 25527233   /usr/lib/gconv/ISO8859-1.so
009ee000-009f1000 r-xp 00000000 fd:00 36897051   /usr/lib/libglibjni-0.2.so
009f1000-009f2000 rwxp 00002000 fd:00 36897051   /usr/lib/libglibjni-0.2.so
00e2c000-00e2d000 r-xp 00e2c000 00:00 0          [vdso]
08048000-0804a000 r-xp 00000000 fd:00 13893726   /notnfs/cagney/native/frysk-gtk/tests/frysk3116/RegionAndGCFailure
0804a000-0804b000 rwxp 00001000 fd:00 13893726   /notnfs/cagney/native/frysk-gtk/tests/frysk3116/RegionAndGCFailure
09e7a000-09fab000 rwxp 09e7a000 00:00 0
48b69000-48b82000 r-xp 00000000 fd:00 46466635   /lib/ld-2.5.so
48b82000-48b83000 r-xp 00018000 fd:00 46466635   /lib/ld-2.5.so
48b83000-48b84000 rwxp 00019000 fd:00 46466635   /lib/ld-2.5.so
48b86000-48cbd000 r-xp 00000000 fd:00 46466636   /lib/libc-2.5.so
48cbd000-48cbf000 r-xp 00137000 fd:00 46466636   /lib/libc-2.5.so
48cbf000-48cc0000 rwxp 00139000 fd:00 46466636   /lib/libc-2.5.so
48cc0000-48cc3000 rwxp 48cc0000 00:00 0
48cc5000-48cc7000 r-xp 00000000 fd:00 46466637   /lib/libdl-2.5.so
48cc7000-48cc8000 r-xp 00001000 fd:00 46466637   /lib/libdl-2.5.so
48cc8000-48cc9000 rwxp 00002000 fd:00 46466637   /lib/libdl-2.5.so
48ccb000-48cf0000 r-xp 00000000 fd:00 46466643   /lib/libm-2.5.so
48cf0000-48cf1000 r-xp 00024000 fd:00 46466643   /lib/libm-2.5.so
48cf1000-48cf2000 rwxp 00025000 fd:00 46466643   /lib/libm-2.5.so
48cf4000-48d07000 r-xp 00000000 fd:00 46466638   /lib/libpthread-2.5.so
48d07000-48d08000 r-xp 00012000 fd:00 46466638   /lib/libpthread-2.5.so
48d08000-48d09000 rwxp 00013000 fd:00 46466638   /lib/libpthread-2.5.so
48d09000-48d0b000 rwxp 48d09000 00:00 0
48d0d000-48d1f000 r-xp 00000000 fd:00 36897469   /usr/lib/libz.so.1.2.3
48d1f000-48d20000 rwxp 00011000 fd:00 36897469   /usr/lib/libz.so.1.2.3
48d22000-48d29000 r-xp 00000000 fd:00 46466639   /lib/librt-2.5.so
48d29000-48d2a000 r-xp 00006000 fd:00 46466639   /lib/librt-2.5.so
48d2a000-48d2b000 rwxp 00007000 fd:00 46466639   /lib/librt-2.5.so
48d2d000-48d31000 r-xp 00000000 fd:00 25470009   /usr/lib/libglibjava-0.2.so
48d31000-48d33000 rwxp 00003000 fd:00 25470009   /usr/lib/libglibjava-0.2.so
48d46000-48de3000 r-xp 00000000 fd:00 46466640   /lib/libglib-2.0.so.0.1200.3
48de3000-48de4000 rwxp 0009c000 fd:00 46466640   /lib/libglib-2.0.so.0.1200.3
48e2e000-48e30000 r-xp 00000000 fd:00 36897473   /usr/lib/libXau.so.6.0.0
48e30000-48e31000 rwxp 00001000 fd:00 36897473   /usr/lib/libXau.so.6.0.0
48e33000-48e380Aborted

valgrind bad free for testcase on FC-6 2006-10-26:

==25731== Invalid free() / delete / delete[]
==25731==    at 0x400501A: free (vg_replace_malloc.c:233)
==25731==    by 0x48D785F0: g_free (gmem.c:187)
==25731==    by 0x4DA1327: processPendingStruct (gtk_java.c:493)
==25731==    by 0x4011591: jg_process_atexit (jg_jnu.c:485)
==25731==    by 0x48BB1938: exit (exit.c:75)
==25731==    by 0x4AE2CC96: _ZN4java4lang7Runtime12exitInternalEJvi (in /usr/lib/libgcj.so.7rh.0.0)
==25731==    by 0x4B1C2A8D: _ZN4java4lang7Runtime12exitNoChecksEJvi (in /usr/lib/libgcj.so.7rh.0.0)
==25731==    by 0x4B1C2AF8: _ZN4java4lang7Runtime20exitNoChecksAccessorEJvi (in /usr/lib/libgcj.so.7rh.0.0)
==25731==    by 0x4AE20579: _ZN3gnu4java4lang10MainThread9call_mainEJvv (in /usr/lib/libgcj.so.7rh.0.0)
==25731==    by 0x4AE859D6: _ZN3gnu4java4lang10MainThread3runEJvv (in /usr/lib/libgcj.so.7rh.0.0)
==25731==    by 0x4AE3040A: _Jv_ThreadRun(java::lang::Thread*) (in /usr/lib/libgcj.so.7rh.0.0)
==25731==    by 0x4ADEB34B: _Jv_RunMain(_Jv_VMInitArgs*, java::lang::Class*, char const*, int, char const**, bool) (in /usr/lib/libgcj.so.7rh.0.0)
==25731==  Address 0x4FC1020 is 32 bytes inside a block of size 504 alloc'd
==25731==    at 0x4004600: memalign (vg_replace_malloc.c:332)
==25731==    by 0x400465A: posix_memalign (vg_replace_malloc.c:421)
==25731==    by 0x48D87B63: slab_allocator_alloc_chunk (gslice.c:1065)
==25731==    by 0x48D885BB: g_slice_alloc (gslice.c:619)
==25731==    by 0x49919843: pango_find_map (in /usr/lib/libpango-1.0.so.0.1400.4)
==25731==    by 0x4991CC39: (within /usr/lib/libpango-1.0.so.0.1400.4)
==25731==    by 0x4991D061: pango_itemize_with_base_dir (in /usr/lib/libpango-1.0.so.0.1400.4)
==25731==    by 0x499250D0: (within /usr/lib/libpango-1.0.so.0.1400.4)
==25731==    by 0x49925CAA: (within /usr/lib/libpango-1.0.so.0.1400.4)
==25731==    by 0x499508A6: pango_fc_font_create_metrics_for_context (in /usr/lib/libpangoft2-1.0.so.0.1400.4)
==25731==    by 0x49981761: (within /usr/lib/libpangocairo-1.0.so.0.1400.4)
==25731==    by 0x499170EE: pango_font_get_metrics (in /usr/lib/libpango-1.0.so.0.1400.4)

(In reply to comment #14)
> Observations:
> 
> 1. Did you know the crash happens [in the test case] at app termination, not
> during the gc() or the sleep()?

Yes, attaching a re-ordered patch that triggers the barf during a more normal "idle".

> 2. The two calls to getClipRegion() return different objects, which surprised
> me. I had been under the impression that there was some logic to lookup and
> return the same object; specifically, glib-java/src/jni/glib_java.c contains
> getGObjectHandle() which seems to try and use an existing object. 
> 
> 3. (2) was evident when I split the
> drawingArea.getClipRegion().getClipbox().getHeight() chains into individual
> statements.

as in:
  tmp1 = drawingArea.getClipRegion();
  tmp2 = tmp1.getHeight();
?

> 4. By doing (3) ie commenting out the orginal chained getters] the crash does
> *NOT* occur. Do the chained getters (again, after the individual statements)
> and the crash returns.

Assuming the above change, and remembering that the bug requires:

- creation of regions via getClipRegion()
- Java Garbage Collect causing those objects to be finalized
- Gnome Event loop run causing the corresponding GNOME objects to be "free"ed

if local variables are added then the System.gc() won't do the finalize() as it will find references to those objects still on the stack.

(In reply to comment #3)

> Anyway, my suggestion would be to try changing Region from a Struct into a
> Boxed and see if that eliminates the bug. That would be s/{Mem,}Struct/Boxed/g
> in
> 
> src/java/org/gnu/gdk/Region.c
> src/jni/org_gnu_gdk_Region.c

Following on from mcvet's comment, It just changed to backtrace to:

/lib/libc.so.6[0x48bebefd]
/lib/libc.so.6(cfree+0x90)[0x48bef550]
/lib/libglib-2.0.so.0(g_free+0x31)[0x48d785f1]
/usr/lib/libgtkjni-2.8.so(processPendingGBoxed+0x98)[0x610328]

i.e., it is still doing the g_free.

Created attachment 75484 [details]
GLIBC crash during GC due to refresh and regions

Simplified test case: makes the call sequence:

- allocate objects
- run garbage collector
- run GNOME idle loop

more explicit

Created attachment 75485 [details] [review]
set free method for regions to gdk_region_destroy, not g_free

Regions being created in Drawable were being assigned a free method of g_free instead of gdk_region_destroy.  Consequently just those, and not any allocated via the region object, were being freed with the wrong method.

Created attachment 75486 [details]
Andrew's slightly refactored test case

For what it's worth, I refactored the test case like this, which is how I noticed that it didn't seem to be happening until app termination. Probably not relevant, but worth having here in case [something like] this appears in the future.

AfC

Andrew,

Since you assigned this to yourself, do you want to commit it the patch or are you indicating you wish to hold it off pending further testing? [Bugzilla does rather have shortcomings as a task manager, doesn't it. Oh well]

Assign it to me if you want me to commit it.

AfC

Yes, I'll check it in; I need to confirm that my CVS is working anyway.

(In reply to comment #23)
> Andrew,
> 
> Since you assigned this to yourself, do you want to commit it the patch or are
> you indicating you wish to hold it off pending further testing? [Bugzilla does
> rather have shortcomings as a task manager, doesn't it. Oh well]
> 
> Assign it to me if you want me to commit it.
> 
> AfC
> 

Fixes committed to both HEAD and stable branches.