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 694920 - Crash when loading http://www.alexa.com/topsites after 89a3db70
Crash when loading http://www.alexa.com/topsites after 89a3db70
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: Misc
unspecified
Other Linux
: High critical
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on:
Blocks:
 
 
Reported: 2013-03-01 09:48 UTC by Sergio Villar
Modified: 2013-03-04 14:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (1.44 KB, patch)
2013-03-04 10:36 UTC, Sergio Villar
committed Details | Review

Description Sergio Villar 2013-03-01 09:48:41 UTC
After bisecting seems like the fix for bug 682527 is causing the crashes. Attaching backtrace although it is not very informative:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff0ae6eb8 in g_hash_table_lookup_node (hash_table=0x51, key=0x85d7b6f1, hash_return=0x7fffffffacc8) at ghash.c:365
365	  hash_value = hash_table->hash_func (key);
(gdb) bt
  • #0 g_hash_table_lookup_node
    at ghash.c line 365
  • #1 g_hash_table_contains
    at ghash.c line 1243
  • #2 assign_source_id_unlocked
    at gmain.c line 1098
  • #3 g_source_attach_unlocked
    at gmain.c line 1113
  • #4 g_source_attach
    at gmain.c line 1162
  • #5 soup_add_completion_reffed
    at soup-misc.c line 125
  • #6 soup_session_real_kick_queue
    at soup-session.c line 2131
  • #7 soup_session_kick_queue
    at soup-session.c line 2147
  • #8 soup_session_send_async
    at soup-session.c line 3995
  • #9 soup_request_http_send_async
  • #10 soup_request_send_async

Comment 1 Sergio Villar 2013-03-01 20:43:08 UTC
This is a memory issue for sure but I can make it stop crashing by just commenting the 

entry->dirty = FALSE;

in istream_finished_cb(). I cannot find another reason but some threads trying to access the same entry at the same time, but don't know how it could happen...
Comment 2 Dan Winship 2013-03-03 18:10:41 UTC
(In reply to comment #1)
> I cannot find another reason but some threads trying
> to access the same entry at the same time, but don't know how it could
> happen...

Hm... SoupCacheInputStream doesn't define close_async(), so it will fall back to the default implementation, which runs the sync close() in another thread. Could that be it?

It looks like SoupCacheInputStream's close_fn() doesn't do anything blocking, so you can implement its close_async() to just call close_fn() and asynchronously return the result
Comment 3 Sergio Villar 2013-03-04 10:11:45 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > I cannot find another reason but some threads trying
> > to access the same entry at the same time, but don't know how it could
> > happen...
> 
> Hm... SoupCacheInputStream doesn't define close_async(), so it will fall back
> to the default implementation, which runs the sync close() in another thread.
> Could that be it?
> 
> It looks like SoupCacheInputStream's close_fn() doesn't do anything blocking,
> so you can implement its close_async() to just call close_fn() and
> asynchronously return the result

I thought about that but I don't think that's the culprit because WebKit uses no the synchronous version of close(), so that could not trigger a new thread.

Good news is that I could get it fail with valgrind (99.9% of the times it does not crash when run in valgrind):

==12555== Invalid read of size 4
==12555==    at 0xC79FD54: pthread_mutex_lock (pthread_mutex_lock.c:50)
==12555==    by 0xBEBF140: g_mutex_lock (gthread-posix.c:210)
==12555==    by 0xBE6D2D8: g_main_context_find_source_by_user_data (gmain.c:2146)
==12555==    by 0x8A8560F: soup_session_real_kick_queue (soup-session.c:2129)
==12555==    by 0x8A856CD: soup_session_kick_queue (soup-session.c:2147)
==12555==    by 0x8A88620: soup_session_send_async (soup-session.c:3995)
==12555==    by 0x8A7CC68: soup_request_http_send_async (soup-request-http.c:143)
==12555==    by 0x8A7B81F: soup_request_send_async (soup-request.c:244)
==12555==    by 0x75CCD55: WebCore::ResourceHandle::start() (in /opt/lib64/libwebkitgtk-3.0.so.0.18.1)
==12555==    by 0x75BD3A3: WebCore::ResourceHandle::create(WebCore::NetworkingContext*, WebCore::ResourceRequest const&, WebCore::ResourceHandleClient*, bool, bool) (in /opt/lib64/libwebkitgtk-3.0.so.0.18.1)
==12555==    by 0x6D51993: WebCore::ResourceLoader::start() (in /opt/lib64/libwebkitgtk-3.0.so.0.18.1)
==12555==    by 0x6D56AE9: WebCore::ResourceLoadScheduler::servePendingRequests(WebCore::ResourceLoadScheduler::HostInformation*, WebCore::ResourceLoadPriority) (in /opt/lib64/libwebkitgtk-3.0.so.0.18.1)
==12555==  Address 0x1d9e6db0 is 16 bytes inside a block of size 40 free'd
==12555==    at 0x4C2AF5C: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12555==    by 0xBEBF084: g_mutex_impl_free (gthread-posix.c:115)
==12555==    by 0xBEBF11E: g_mutex_clear (gthread-posix.c:189)
==12555==    by 0xBE6AFA6: g_main_context_unref (gmain.c:539)
==12555==    by 0xBE6EF90: g_main_loop_unref (gmain.c:3833)
==12555==    by 0x75CEC62: WebCore::WebCoreSynchronousLoader::~WebCoreSynchronousLoader() (in /opt/lib64/libwebkitgtk-3.0.so.0.18.1)
==12555==    by 0x75CBCD9: WebCore::ResourceHandle::platformLoadResourceSynchronously(WebCore::NetworkingContext*, WebCore::ResourceRequest const&, WebCore::StoredCredentials, WebCore::ResourceError&, WebCore::ResourceResponse&, WTF::Vector<char, 0ul>&) (in /opt/lib64/libwebkitgtk-3.0.so.0.18.1)
==12555==    by 0x75BCAFC: WebCore::ResourceHandle::loadResourceSynchronously(WebCore::NetworkingContext*, WebCore::ResourceRequest const&, WebCore::StoredCredentials, WebCore::ResourceError&, WebCore::ResourceResponse&, WTF::Vector<char, 0ul>&) (in /opt/lib64/libwebkitgtk-3.0.so.0.18.1)
==12555==    by 0x6D198D1: WebCore::FrameLoader::loadResourceSynchronously(WebCore::ResourceRequest const&, WebCore::StoredCredentials, WebCore::ResourceError&, WebCore::ResourceResponse&, WTF::Vector<char, 0ul>&) (in /opt/lib64/libwebkitgtk-3.0.so.0.18.1)
==12555==    by 0x6D0999B: WebCore::DocumentThreadableLoader::loadRequest(WebCore::ResourceRequest const&, WebCore::SecurityCheckPolicy) (in /opt/lib64/libwebkitgtk-3.0.so.0.18.1)

So I think my commit might be unveiling some other already present bug, which is that basically we're trying to use an already unref'ed main context (each synchronous load in WK create a new main context).
Comment 4 Sergio Villar 2013-03-04 10:36:27 UTC
Created attachment 237944 [details] [review]
Patch

Keeping our own reference of the GMainContext fixes the crash as we'll have a reference to it at least until the queue item is freed.
Comment 5 Dan Winship 2013-03-04 13:45:31 UTC
Comment on attachment 237944 [details] [review]
Patch

>+	if (item->async_context)
>+		g_main_context_ref (item->async_context);

move that to right after the item->async_context = ... line

otherwise good. thanks
Comment 6 Sergio Villar 2013-03-04 14:38:32 UTC
Committed to master.