[Webkit-unassigned] [Bug 144262] [GTK] Crash in WebProcess when loading large content with custom URI schemes

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=144262

--- Comment #3 from Mario Sanchez Prada <mario at webkit.org> ---
To keep up with my longest bug report ever, here comes a summary of my investigation so far:

Basically, it seems to me that the problem is that the implementation of CustomProtocolManagerImpl::didFailWithError() in the Web process is assuming that the GTask has not been released yet when invoked, which seems like is not always the case (either because of a bigger design issue, or a failing implementation of this method).

To understand the situation better, I spent some time trying to understand how events seem to develop in this scenario, and this is the summary:

First of all, when the UI process sends the DidLoad message to the CustomProtocolmanager in the Web process for the first time, data->stream is NULL in CustomProtocolManagerImpl::didLoadData() when called, which is perfectly normal at that point. Then, precisely because it's the first time didLoad() gets called, data->stream gets filled for the first time too with data coming from the UI process, so that the loader can start processing it, and the GTask inside the WebSoupRequestAsyncData object gets released, since it's no longer needed (it's purpose seems to be to setup this data->stream object for the first time).

But now, because async reads from the source input stream happen very fast in the UI process, at least a second DidLoad message got probably sent to the Web process, even if the first load gets cancelled very quickly because of the second call to webkit_web_view_load_uri().

When this occurs, the second time CustomProtocolManagerImpl::didLoad() gets executed, the statement in "if (data->requestFailed())" will be true (due to the cancelled loading operation), meaning that a StopLoading message will be sent back to the UI process, to notify it that it should stop sending data.

But when this StopLoading message arrives in the UI process, the GCancellable used in WebKitURISchemeRequest to initiate the asynchronous load of data (via g_input_stream_read_async()) gets cancelled as well, meaning that further calls to the webkitURISchemeRequestReadCallback() callback will get a GError when calling g_input_stream_read_finish(), which will end up getting webkit_uri_scheme_request_finish_error() executed.

And what will webkit_uri_scheme_request_finish_error() do now? Send a DidFailWithError message to the Web process, which will be handled in CustomProtocolManagerImpl::didFailWithError().

Now, effectively, this method is being executed **after** the GTask object were released for a certain value customProtocolID, back when data->stream was set up for the first time. And because the implementation is something like this: 

  void CustomProtocolManagerImpl::didFailWithError(uint64_t customProtocolID, const WebCore::ResourceError& error)
  {
      WebSoupRequestAsyncData* data = m_customProtocolMap.get(customProtocolID);
      ASSERT(data);

      GRefPtr<GTask> task = data->releaseTask();
      ASSERT(task.get());
      g_task_return_new_error(task.get(), g_quark_from_string(error.domain().utf8().data()),
                              error.errorCode(), "%s", error.localizedDescription().utf8().data());

      m_customProtocolMap.remove(customProtocolID);
  }

... we get the crash, since data->releaseTask() has been called earlier and therefore there's nothing to complete at this point.

So, it looks to me that either the whole logic of how didLoad() and didFailWithError() needs "some adjustment", or the didFailError() needs to be fixed so that it handles this situation gracefully, while still being correct.

Comments?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150427/7957d29e/attachment.html>