Summary: | REGRESSION (r286601): storage/filesystemaccess/sync-access-handle-read-write-worker.html and file-system-access/sandboxed_FileSystemSyncAccessHandle-truncate.https.tentative.worker.html are consistently failing | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryan Haddad <ryanhaddad> | ||||||||||||||
Component: | WebCore Misc. | Assignee: | Sihui Liu <sihui_liu> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | annulen, ews-watchlist, gyuyoung.kim, jenner, ryuan.choi, sergio, sihui_liu, webkit-bot-watchers-bugzilla, webkit-bug-importer, youennf | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=233567 | ||||||||||||||||
Attachments: |
|
Description
Ryan Haddad
2021-12-13 14:27:16 PST
*** Bug 234267 has been marked as a duplicate of this bug. *** Created attachment 447115 [details]
Patch
Comment on attachment 447115 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447115&action=review > Source/WebCore/Modules/filesystemaccess/FileSystemDirectoryHandle.cpp:60 > + promise.resolve(FileSystemFileHandle::create(context, String { name }, result.returnValue(), WTFMove(connection))); I am not sure we can guarantee that the task given to workerRunLoop.postTask in WorkerFileSystemStorageConnection::getFileHandle/getDirectoryHandle will always be executed. Say we called worker.stop() and a few milliseconds later we call workerRunLoop::postTask. Maybe the best approach would be for FileSystemStorageConnection::getFileHandle/getDirectoryHandle to return in the completion handler an allocated object that would have the sync handle identifier and a weakref to the IPC connection. This object would, when destroyed, call the sync handle using connection and identifier. Then either FileSystemFileHandle would store this object until destroyed, or FileSystemFileHandle could steal the identifier and clear the connection so that the object destructor would do nothing in that case. Created attachment 447174 [details]
Patch
Comment on attachment 447174 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447174&action=review > Source/WebCore/Modules/filesystemaccess/WorkerFileSystemStorageConnection.cpp:37 > +class FileSystemHandleCloseScope : public ThreadSafeRefCounted<FileSystemHandleCloseScope> { Not sure it needs to be refcounted, maybe we can just have a unique_ptr> Also, maybe we should expose this object in WebCore and callbacks with a needed WebProcess specialisation to do the clean-up. That way, we would not need to modify the code to allow FileSystemFileHandle creation with a null context in WebCore. > Source/WebCore/Modules/filesystemaccess/WorkerFileSystemStorageConnection.cpp:47 > + m_connection->closeHandle(m_identifier); I am not sure closeHandle is expected to be called from a non main thread. You might fix this by explicitly call callOnMainThread, or make ThreadSafeRefCounted destruction main run loop. > Source/WebCore/Modules/filesystemaccess/WorkerFileSystemStorageConnection.cpp:168 > + return; We are doing twice the same "is scope closed" here and in the WebCore callback. Created attachment 447331 [details]
Patch
Created attachment 447332 [details]
Patch
Created attachment 447335 [details]
Patch
Created attachment 447363 [details]
Patch
Committed r287156 (245333@main): <https://commits.webkit.org/245333@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447363 [details]. |