[GPUP] Ignore an IPC message if the message receiver has been destroyed
Created attachment 420358 [details] Patch
Comment on attachment 420358 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420358&action=review > Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:110 > +Logger& GPUProcessConnection::logger() > +{ > + if (!m_logger) > + m_logger = Logger::create(this); > + > + return *m_logger; > +} > + It looks like this is unused, `RELEASE_LOG_ERROR` does not use a Logger.
Comment on attachment 420358 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420358&action=review >> Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:110 >> + > > It looks like this is unused, `RELEASE_LOG_ERROR` does not use a Logger. Right, will fix it. Thanks!
Created attachment 420374 [details] Revise the patch based on Eric's comment
Committed r272887: <https://commits.webkit.org/r272887> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420374 [details].
<rdar://problem/74369181>
We are still seeing these two tests crashing even after this change: imported/w3c/web-platform-tests/html/canvas/element/imagebitmap/createImageBitmap-origin.sub.html http/tests/navigation/parsed-iframe-dynamic-form-back-entry.html history: https://results.webkit.org/?suite=layout-tests&suite=layout-tests&test=http%2Ftests%2Fnavigation%2Fparsed-iframe-dynamic-form-back-entry.html&test=imported%2Fw3c%2Fweb-platform-tests%2Fhtml%2Fcanvas%2Felement%2Fimagebitmap%2FcreateImageBitmap-origin.sub.html
Crash log for imported/w3c/web-platform-tests/html/canvas/element/imagebitmap/createImageBitmap-origin.sub.html https://build.webkit.org/results/Apple-BigSur-Debug-WK2-Tests/r272998%20(200)/imported/w3c/web-platform-tests/html/canvas/element/imagebitmap/createImageBitmap-origin.sub-crash-log.txt Crash log for http/tests/navigation/parsed-iframe-dynamic-form-back-entry.html https://build.webkit.org/results/Apple-BigSur-Debug-WK2-Tests/r272998%20(200)/http/tests/navigation/parsed-iframe-dynamic-form-back-entry-crash-log.txt
Reopening for continuing issues
(In reply to Truitt Savell from comment #8) > Crash log for > imported/w3c/web-platform-tests/html/canvas/element/imagebitmap/ > createImageBitmap-origin.sub.html > > https://build.webkit.org/results/Apple-BigSur-Debug-WK2-Tests/ > r272998%20(200)/imported/w3c/web-platform-tests/html/canvas/element/ > imagebitmap/createImageBitmap-origin.sub-crash-log.txt > > > Crash log for > http/tests/navigation/parsed-iframe-dynamic-form-back-entry.html > > https://build.webkit.org/results/Apple-BigSur-Debug-WK2-Tests/ > r272998%20(200)/http/tests/navigation/parsed-iframe-dynamic-form-back-entry- > crash-log.txt These are GPU Process crashes.
Filed bug 222079 to fix the crash of http/tests/navigation/parsed-iframe-dynamic-form-back-entry.html.
Bug 221989 tracks the crash of imported/w3c/web-platform-tests/html/canvas/element/imagebitmap/createImageBitmap-origin.sub.html.
Let's close this bug.
Comment on attachment 420374 [details] Revise the patch based on Eric's comment View in context: https://bugs.webkit.org/attachment.cgi?id=420374&action=review > Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:181 > + RELEASE_LOG_ERROR(WebGL, "The RemoteGraphicsContextGLProxy object has beed destroyed"); From WebGL perspective this is incorrect. This is not an error. This is expected, the sender cannot know when the receiver decides to stop listening.
Comment on attachment 420374 [details] Revise the patch based on Eric's comment View in context: https://bugs.webkit.org/attachment.cgi?id=420374&action=review >> Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:181 >> + RELEASE_LOG_ERROR(WebGL, "The RemoteGraphicsContextGLProxy object has beed destroyed"); > > From WebGL perspective this is incorrect. > This is not an error. This is expected, the sender cannot know when the receiver decides to stop listening. Hmm, but this log message is from GPUProcessConnection's perspective?
(In reply to Kimmo Kinnunen from comment #14) > Comment on attachment 420374 [details] > Revise the patch based on Eric's comment > > View in context: > https://bugs.webkit.org/attachment.cgi?id=420374&action=review > > > Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:181 > > + RELEASE_LOG_ERROR(WebGL, "The RemoteGraphicsContextGLProxy object has beed destroyed"); > > From WebGL perspective this is incorrect. > This is not an error. This is expected, the sender cannot know when the > receiver decides to stop listening. This is immaterial to whether the receiver is a WebGL object or not; this is a bug in our XPC implementation. And it's not true that the sender cannot know when the receiver decides to stop listening. The receiver can send an async message to the remote process and wait for a response before stopping listening.
(In reply to Jer Noble from comment #16) > And it's not true that the sender cannot > know when the receiver decides to stop listening. The receiver can send an > async message to the remote process and wait for a response before stopping > listening. Web process sends "do something" for destination id 7. GPU process receives "do something". GPU process sends "something done". Web process sends "do something" else for destination id 7. Web process notices GPU process timed out. Web process removes all references to anything related to id 7. Web process receives "something done" for id 7. In general, in asynchronous communication you cannot know if the other party will get the message you send.
> The receiver can send an > async message to the remote process and wait for a response before stopping > listening. The destruction is not a sync operation. Would it be useful as as sync operation? Here's a sequence that might trigger the error message WebContent: Send CreateRemoteGraphicsContextGL id 7 Send ReleaseRemoteGraphicsContextGL id 7 GPU Process: Got CreateRemoteGraphicsContextGL id 7 Send WasCreated id 7 Got ReleaseRemoteGraphicsContextGL id 7
(In reply to Kimmo Kinnunen from comment #18) > The destruction is not a sync operation. Would it be useful as as sync > operation? Sync or async with a reply would fit here better -- does not matter. Destruction is not usually something you compute a result with.