| Summary: | [GTK] fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html is crashing | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Zan Dobersek <zan> |
| Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW --- | ||
| Severity: | Normal | CC: | bugs-noreply, danw, d-r, gustavo, mrobinson |
| Priority: | P2 | Keywords: | Gtk, LayoutTestFailure |
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
|
Description
Zan Dobersek
2012-12-11 10:35:35 PST
The 32-bit and 64-bit WK2 bots caught up and are reporting constant crashes: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=fast%2Fxmlhttprequest%2Fxmlhttprequest-recursive-sync No crashes on the debug builder, though. The crashes are now occurring on the debug builder as well. Bug #100117 has detailed explanation of how this test works and why it can crash. Basically, on GTK port, the maximum call stack size is never exceeded so the file descriptors get out of hand in libsoup. EFL and Qt ports are skipping the test, treating it like a WONTFIX. IMO crashing is bad, so this test still needs attention on the GTK port. Eventually, we should switch to using the new "plain" SoupSession (rather than SoupSessionAsync), which lets you do both blocking and async I/O with the same session, and then we can just do blocking I/O here rather than async-I/O-on-a-new-GMainContext, so we wouldn't use an extra fd per sync request. There might be other issues with that though that will take some time to iron out, so an easier short-term fix would be to add our own xmlhttprequest nesting limit and throw StackOverflowError earlier than we normally would. The test started failing on the 64-bit debug builder after the swap on that system was increased in size. It seems that caused an increase in the stack size as well, causing this test to crash but other test failures as well (bug #107257). At the moment I don't have any clear idea of how to approach that. In the case of this test and the current soup behavior, this seems as an occurrence that any user could stumble upon. (In reply to comment #3) > Eventually, we should switch to using the new "plain" SoupSession (rather than SoupSessionAsync), which lets you do both blocking and async I/O with the same session, and then we can just do blocking I/O here rather than async-I/O-on-a-new-GMainContext, so we wouldn't use an extra fd per sync request. That seems ideal. > There might be other issues with that though that will take some time to iron out, so an easier short-term fix would be to add our own xmlhttprequest nesting limit and throw StackOverflowError earlier than we normally would. I'm not sure if that is doable. The stack is controlled solely in JavaScriptCore, with (to me) no clear way of limiting the XHR nesting count from WebCore. (In reply to comment #4) > > There might be other issues with that though that will take some time to iron out, so an easier short-term fix would be to add our own xmlhttprequest nesting limit and throw StackOverflowError earlier than we normally would. > > I'm not sure if that is doable. The stack is controlled solely in JavaScriptCore, with (to me) no clear way of limiting the XHR nesting count from WebCore. Well, I was thinking maybe you could return an appropriate error from loadResourceSynchronously that would get translated to StackOverflowError... However... I notice now that loadResourceSynchronously asserts !loadingSynchronousRequest, which means that the previous WebCoreSynchronousLoader has to have already been destroyed at that point... so we shouldn't run out of fds, unless we're leaking GMainContexts or something? (In reply to comment #3) > Eventually, we should switch to using the new "plain" SoupSession (rather than SoupSessionAsync), which lets you do both blocking and async I/O with the same session, and then we can just do blocking I/O here rather than async-I/O-on-a-new-GMainContext, so we wouldn't use an extra fd per sync request. I created bug 107451 to track that suggestion. |