Created attachment 423293 [details] http/tests/xmlhttprequest/upload-onloadstart-event.html crash log http/tests/xmlhttprequest/loadstart-event-init.html http/tests/xmlhttprequest/redirect-cross-origin-post-sync.html http/tests/xmlhttprequest/redirect-cross-origin-post.html http/tests/xmlhttprequest/response-access-on-error.html http/tests/xmlhttprequest/upload-onabort-progressevent-attributes.html http/tests/xmlhttprequest/upload-onload-event.html http/tests/xmlhttprequest/upload-onload-progressevent-attributes.html http/tests/xmlhttprequest/upload-onloadend-event-after-abort.html http/tests/xmlhttprequest/upload-onloadend-event-after-load.html http/tests/xmlhttprequest/upload-onloadstart-event.html http/tests/xmlhttprequest/upload-onprogress-event.html http/tests/xmlhttprequest/upload-progress-events.html http/tests/xmlhttprequest/xmlhttprequest-sync-no-progress-events.html These happen to crash locally and in the bots (althought not all might be run, due to early exits). This is only a partial list. Other tests might be affected. STDERR: STDERR: (process:2504): libsoup-CRITICAL **: 20:42:47.243: soup_message_set_request_body: assertion 'stream == NULL || G_IS_POLLABLE_INPUT_STREAM (stream)' failed STDERR: LEAK: 1 WebPageProxy Tip of trace for http/tests/xmlhttprequest/upload-onloadstart-event.html (full attached) Thread 1 (Thread 0x7efdae91e9c0 (LWP 2504)): #0 g_logv (log_domain=0x7efdafbb7943 "libsoup", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=<optimized out>) at ../glib/gmessages.c:1413 #1 0x00007efdb0eb4973 in g_log (log_domain=<optimized out>, log_level=<optimized out>, format=<optimized out>) at ../glib/gmessages.c:1451 #2 0x00007efdbc2257f6 in WebCore::ResourceRequest::updateSoupMessageBody(_SoupMessage*, WebCore::BlobRegistryImpl&) const (this=0x7efd6c2f3788, soupMessage=0x7efd4c0083b0 [SoupMessage], blobRegistry=... ) at ../../Source/WebCore/platform/network/soup/ResourceRequestSoup.cpp:138 #3 0x00007efdbc22556c in WebCore::ResourceRequest::createSoupMessage(WebCore::BlobRegistryImpl&) const (this=0x7efd6c2f3788, blobRegistry=...) at ../../Source/WebCore/platform/network/soup/ResourceReque stSoup.cpp:90 #4 0x00007efdb8c06b17 in WebKit::NetworkDataTaskSoup::createRequest(WebCore::ResourceRequest&&, WebKit::NetworkDataTaskSoup::WasBlockingCookies) (this=0x7efd6c2f3580, request=..., wasBlockingCookies=Web Kit::NetworkDataTaskSoup::WasBlockingCookies::No) at ../../Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp:127 #5 0x00007efdb8c065f1 in WebKit::NetworkDataTaskSoup::NetworkDataTaskSoup(WebKit::NetworkSession&, WebKit::NetworkDataTaskClient&, WebCore::ResourceRequest const&, WTF::ObjectIdentifier<WebCore::FrameId entifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::StoredCredentialsPolicy, WebCore::ContentSniffingPolicy, WebCore::ContentEncodingSniffingPolicy, bool, bool) (this=0x7efd6c2f35 80, session=..., client=..., requestWithCredentials=..., frameID=..., pageID=..., storedCredentialsPolicy=WebCore::StoredCredentialsPolicy::Use, shouldContentSniff=WebCore::ContentSniffingPolicy::DoNotSniffContent, shouldClearReferrerOnHTTPSToHTTPRedirect=true, dataTaskIsForMainFrameNavigation=false) at ../../Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp:81 #6 0x00007efdb88c1b57 in WebKit::NetworkDataTaskSoup::create(WebKit::NetworkSession&, WebKit::NetworkDataTaskClient&, WebCore::ResourceRequest const&, WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::StoredCredentialsPolicy, WebCore::ContentSniffingPolicy, WebCore::ContentEncodingSniffingPolicy, bool, bool) (session=..., client=..., request=..., frameID=..., pageID=..., storedCredentialsPolicy=WebCore::StoredCredentialsPolicy::Use, shouldContentSniff=WebCore::ContentSniffingPolicy::DoNotSniffContent, shouldContentEncodingSniff=WebCore::ContentEncodingSniffingPolicy::DoNotSniff, shouldClearReferrerOnHTTPSToHTTPRedirect=true, dataTaskIsForMainFrameNavigation=false) at ../../Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.h:45 #7 0x00007efdb88acb5d in WebKit::NetworkDataTask::create(WebKit::NetworkSession&, WebKit::NetworkDataTaskClient&, WebKit::NetworkLoadParameters const&) (session=..., client=..., parameters=...) at ../../Source/WebKit/NetworkProcess/NetworkDataTask.cpp:60 #8 0x00007efdb88b015b in WebKit::NetworkLoad::NetworkLoad(WebKit::NetworkLoadClient&, WebCore::BlobRegistryImpl*, WebKit::NetworkLoadParameters&&, WebKit::NetworkSession&) (this=0x7efdae0e1000, client=..., blobRegistry=0x7efdae0d6138, parameters=..., networkSession=...) at ../../Source/WebKit/NetworkProcess/NetworkLoad.cpp:57
caused by: https://gitlab.gnome.org/GNOME/libsoup/-/commit/0a34d642a401c5880ec4f533250fdddc6dbb12b0
the current soup code doesn't really have any requirement for the request body to be pollable, so the soup3 webkit code assumes that it's not necessary, but the upcoming http2 code will require it to be pollable - so the restriction was put in place - so i guess the webkit code will need to be updated
i guess the thing to do here would be to just do the same as the soup2 code - i.e. read all into bytes, then set the request body from bytes rather than from the input stream...
(In reply to Daniel Kolesa from comment #3) > i guess the thing to do here would be to just do the same as the soup2 code > - i.e. read all into bytes, then set the request body from bytes rather than > from the input stream... That's problematic for long file uploads, for example. The WebKit stream can't be pollable because it used GFileInputStream internally that is not pollable. I can make it pollable and only return TRUE from can_poll when not having file streams. But the solution would be to make libsoup work without pollable streams, at least having fallback code that reads using the async apis into an intermediate buffer that is later used to write in the output.
Created attachment 423468 [details] Patch
patch looks good to me, the rest of the fix needs to be in soup (remove the pollable stream requirement and implement a fallback path for when it's not in the http2 backend)
Comment on attachment 423468 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423468&action=review > Source/WebCore/platform/network/soup/WebKitFormDataInputStream.cpp:137 > + // GFileInputStream is not pollable, so the stream is only pollable if FormData doesn't contain EncodedFileData elements. I wonder if it could be an options to check whether a GInputStream is a GUnixInputStream (which does implement GPollableInputStream) which would make the file elements pollable, or whether the GInputStream implements the GFileDescriptorBased interface, in which case we could use the _get_fd() method and use GPollFD or similar 🤔️ (No need to change anything in this patch, by the way, this is just me mulling ideas over. If you think any of this makes sense for a follow-up patch it would be nice to write it down as a TODO and/or make a new Bugzilla issue to track it.) > Source/WebCore/platform/network/soup/WebKitFormDataInputStream.cpp:177 > + return g_pollable_source_new_full(stream, base.get(), cancellable); Aren't these two lines equivalent to a plain g_pollable_input_stream_create_source() call? As in: return g_pollable_input_stream_create_source(stream, cancellable);
Comment on attachment 423468 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423468&action=review >> Source/WebCore/platform/network/soup/WebKitFormDataInputStream.cpp:137 >> + // GFileInputStream is not pollable, so the stream is only pollable if FormData doesn't contain EncodedFileData elements. > > I wonder if it could be an options to check whether a GInputStream is a GUnixInputStream > (which does implement GPollableInputStream) which would make the file elements pollable, > or whether the GInputStream implements the GFileDescriptorBased interface, in which case > we could use the _get_fd() method and use GPollFD or similar 🤔️ > > (No need to change anything in this patch, by the way, this is just me mulling ideas > over. If you think any of this makes sense for a follow-up patch it would be nice > to write it down as a TODO and/or make a new Bugzilla issue to track it.) We are always reading local files, so it's always GFileInputStream and not pollable. >> Source/WebCore/platform/network/soup/WebKitFormDataInputStream.cpp:177 >> + return g_pollable_source_new_full(stream, base.get(), cancellable); > > Aren't these two lines equivalent to a plain g_pollable_input_stream_create_source() call? > As in: > > return g_pollable_input_stream_create_source(stream, cancellable); This is the implementation of g_pollable_input_stream_create_source().
(In reply to Carlos Garcia Campos from comment #8) > Comment on attachment 423468 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=423468&action=review > > >> Source/WebCore/platform/network/soup/WebKitFormDataInputStream.cpp:137 > >> + // GFileInputStream is not pollable, so the stream is only pollable if FormData doesn't contain EncodedFileData elements. > > > > I wonder if it could be an options to check whether a GInputStream is a GUnixInputStream > > (which does implement GPollableInputStream) which would make the file elements pollable, > > or whether the GInputStream implements the GFileDescriptorBased interface, in which case > > we could use the _get_fd() method and use GPollFD or similar 🤔️ > > > > (No need to change anything in this patch, by the way, this is just me mulling ideas > > over. If you think any of this makes sense for a follow-up patch it would be nice > > to write it down as a TODO and/or make a new Bugzilla issue to track it.) > > We are always reading local files, so it's always GFileInputStream and not > pollable. 👌️ > >> Source/WebCore/platform/network/soup/WebKitFormDataInputStream.cpp:177 > >> + return g_pollable_source_new_full(stream, base.get(), cancellable); > > > > Aren't these two lines equivalent to a plain g_pollable_input_stream_create_source() call? > > As in: > > > > return g_pollable_input_stream_create_source(stream, cancellable); > > This is the implementation of g_pollable_input_stream_create_source(). Right, I did not notice at first, my brain is a bit foggy this morning 😇️
Committed r274640 (235467@main): <https://commits.webkit.org/235467@main>
<rdar://problem/75567967>