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 744862 - tests: fix a race in context-test
tests: fix a race in context-test
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: API
unspecified
Other All
: Normal normal
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on:
Blocks:
 
 
Reported: 2015-02-20 17:31 UTC by Marc-Andre Lureau
Modified: 2015-02-28 15:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tests: fix a race in context-test (2.05 KB, patch)
2015-02-20 17:31 UTC, Marc-Andre Lureau
none Details | Review

Description Marc-Andre Lureau 2015-02-20 17:31:18 UTC
Keep the server context alive as long as the server is alive,
to avoid this kind of race:

 ==31858== Invalid read of size 8
 ==31858==    at 0x5D51B86: find_source_list_for_priority (gmain.c:957)
 ==31858==    by 0x5D51B86: source_remove_from_context (gmain.c:1042)
 ==31858==    by 0x5D51CD2: g_source_unref_internal (gmain.c:2007)
 ==31858==    by 0x4EA5766: soup_add_completion (soup-misc.c:152)
 ==31858==    by 0x405849: soup_test_server_quit_unref (test-utils.c:503)
 ==31858==    by 0x404684: main (context-test.c:353)
 ==31858==  Address 0x7acb760 is 80 bytes inside a block of size 184 free'd
 ==31858==    at 0x4C2ACE9: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
 ==31858==    by 0x5D5A7FE: g_free (gmem.c:190)
 ==31858==    by 0x5D53057: g_main_context_unref (gmain.c:562)
 ==31858==    by 0x4051BD: run_server_thread (test-utils.c:317)
 ==31858==    by 0x5D7B7B4: g_thread_proxy (gthread.c:764)
 ==31858==    by 0x6D4F529: start_thread (pthread_create.c:310)
 ==31858==    by 0x614479C: clone (clone.S:109)
Comment 1 Marc-Andre Lureau 2015-02-20 17:31:22 UTC
Created attachment 297452 [details] [review]
tests: fix a race in context-test
Comment 2 Marc-Andre Lureau 2015-02-20 17:32:20 UTC
(this is different from bug 744861)
Comment 3 Dan Winship 2015-02-20 20:11:24 UTC
Comment on attachment 297452 [details] [review]
tests: fix a race in context-test

Hm... before 2.48, there was no problem here, because the GMainContext would have been set as the SoupServer's :async-context property, and so the SoupServer would hold a ref on it until it was destroyed (just like with your patch).

With the new 2.48 APIs, SoupServer uses the thread-default GMainContext, but never holds a ref on it, so we get the bug. But it seems to me that what test-utils is doing is perfectly reasonable, and ought to work. So I think we should fix this in SoupServer (or SoupSocket) by making it keep a ref on the thread-default context in this case.
Comment 4 Dan Winship 2015-02-28 15:24:30 UTC
fixed in master