Skip to content

subprocess: Fix communicate_cancelled signature

Marco Trevisan requested to merge 3v1n0/glib:subprocess-cancellable-callback-fix into master

The source callback for a GCancellable should have the cancellable itself as first argument. This was not the case, and when this code was hit, we were instead trying to treat the pointer as a CommunicateState reference and thus wrongly deferencing it, causing a memory error and a crash.

This is the valgrind report I had with my simple program (I can provide an example if needed, but I guess this is quite easy to catch):

==15911== Invalid read of size 8
==15911==    at 0x4ED6D14: g_subprocess_communicate_cancelled (gsubprocess.c:1535)
==15911==    by 0x54958A4: g_main_dispatch (gmain.c:3182)
==15911==    by 0x54958A4: g_main_context_dispatch (gmain.c:3847)
==15911==    by 0x5495C6F: g_main_context_iterate.isra.26 (gmain.c:3920)
==15911==    by 0x5495CFB: g_main_context_iteration (gmain.c:3981)
==15911==    by 0x4F056C9: g_application_run (gapplication.c:2494)
==15911==    by 0x1102A9: _vala_main (search-provider.vala:283)
==15911==    by 0x5785B96: (below main) (libc-start.c:310)
==15911==  Address 0x6cc0008 is 16 bytes after a block of size 72 alloc'd
==15911==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==15911==    by 0x549B1F8: g_malloc (gmem.c:99)
==15911==    by 0x54B3965: g_slice_alloc (gslice.c:1024)
==15911==    by 0x54B3E18: g_slice_alloc0 (gslice.c:1050)
==15911==    by 0x52282C3: g_type_create_instance (gtype.c:1836)
==15911==    by 0x52084A7: g_object_new_internal (gobject.c:1805)
==15911==    by 0x5209C74: g_object_new_with_properties (gobject.c:1973)
==15911==    by 0x520A6F0: g_object_new (gobject.c:1645)
==15911==    by 0x1100DC: search_provider_instance_init (search-provider.vala:54)
==15911==    by 0x5228242: g_type_create_instance (gtype.c:1864)
==15911==    by 0x52084A7: g_object_new_internal (gobject.c:1805)
==15911==    by 0x5209C74: g_object_new_with_properties (gobject.c:1973)
==15911==

While as per gdb it was

#0  g_cancellable_cancel (cancellable=0x6) at ../../glib/gio/gcancellable.c:486
#1  0x00007ffff7ab8d1d in g_subprocess_communicate_cancelled (user_data=<optimized out>) at ../../glib/gio/gsubprocess.c:1535

The same can be achivied by instead just changing the signature as g_subprocess_communicate_cancelled (GCancellable *) (or gpointer + inside function cast) and setting the callback with no user_data.

However, I kept this form as I preferred passing the state as always is done around, but let me know if my assumption is not the preferred one.

Merge request reports