Crash in icontainer_real_child_remove on at least 8.5.1 and 8.6.0

[bgilbert]

Fedora has received several reports of segfaults in icontainer_real_child_remove on Fedora 27 (8.5.1) and Fedora 28 (8.6.0).

• Backtraces (note, two slightly different traces)
• Bug report

[mhirsch]

I thought I'd note here that I also see this on Fedora 29, with nip2 8.6.0. Is this fedora specific?

[mhirsch]

I pulled from git just now and built it from source on Fedora 29 and get the same segfault.

[jcupitt]

Thanks for the report Benjamin.

Matt, the bug report seems to suggest this happens simply when launching the program, is that right?

[mhirsch]

Yes, that's right. I tried modifying icontainer_real_child_remove to return when passed a NULL child and was able to start nip2, but found additional segfaults when opening an image.

[jcupitt]

My guess would be something in glib has changed, and that's broken some assumption that nip2 is making about ordering. It's odd I've not seen this in Ubuntu.

I have:

$ pkg-config glib-2.0 --modversion
2.58.1

I'll try fedora28 in docker and see if I can make it happen there.

[mhirsch]

I have

$ pkg-config glib-2.0 --modversion
2.58.1

On Fedora 29.

Thanks for looking into this!

[jcupitt]

I made a debug nip2 for fedora28:

https://github.com/jcupitt/docker-builds/tree/master/nip2-fedora28

I see:

# gdb nip2
(gdb) run
(nip2:23): GLib-GObject-CRITICAL **: 13:27:22.769: g_value_get_pointer: assertion 'G_VALUE_HOLDS_POINTER (value)' failed

Program received signal SIGSEGV, Segmentation fault.
0x000000000043a255 in icontainer_real_child_remove (parent=0xa75400, child=0x0)
    at icontainer.c:645
645		iContainerClass *icontainer_child_class = ICONTAINER_GET_CLASS( child );

So I see the problem, I have a debugger, and I have symbols! I'll see if I can find the cause.

[jcupitt]

I think I've fixed it -- I was being lazy in icontainer and declaring a signal as taking a pointer argument, when actually it was an object. This is harmless on Ubuntu, but it seems Fedora has configured gobject with some extra run-time type checks that flag this and NULL out pointers, causing the crash. This branch now works for me in that docker container anyway,

I'm not sure where to apply this patch. It could potentially be pushed out all the way back to 8.5. What do you think, @bgilbert?

[mhirsch]

That's great news! After applying this patch is it possible to open an image in nip2? I wonder if there are similar pointer/object declarations, since I encountered other crashes after modifying that function to ignore null pointers.

[jcupitt]

I searched for similar problems and fixed them all, but you're right, I didn't actually test opening an image (duh).

I'll have a go.

[jcupitt]

Yes, it loads images fine \o/

[jcupitt]

OK, merged to master, I'll put out 8.7.1 with this fix in a few days. We can backport easily if necessary. Thank you for opening the issue!