Diff
Modified: trunk/LayoutTests/ChangeLog (284691 => 284692)
--- trunk/LayoutTests/ChangeLog 2021-10-22 16:47:52 UTC (rev 284691)
+++ trunk/LayoutTests/ChangeLog 2021-10-22 16:52:26 UTC (rev 284692)
@@ -1,3 +1,18 @@
+2021-10-22 Sihui Liu <sihui_...@apple.com>
+
+ Followup to r284652: ensure file handle is closed in web process
+ https://bugs.webkit.org/show_bug.cgi?id=232127
+
+ Reviewed by Youenn Fablet.
+
+ * storage/filesystemaccess/resources/sync-access-handle-close.js:
+ (testSyncFunction):
+ (async testAsyncFunction):
+ (async testFunctions):
+ (async testMultipleHandles):
+ (async test):
+ * storage/filesystemaccess/sync-access-handle-close-worker-expected.txt:
+
2021-10-22 Chris Dumez <cdu...@apple.com>
[ iOS15 iPad Debug ] imported/w3c/web-platform-tests/webmessaging/MessagePort_onmessage_start.any.worker.html is a flaky failure (reached unreachable code)
Modified: trunk/LayoutTests/storage/filesystemaccess/resources/sync-access-handle-close.js (284691 => 284692)
--- trunk/LayoutTests/storage/filesystemaccess/resources/sync-access-handle-close.js 2021-10-22 16:47:52 UTC (rev 284691)
+++ trunk/LayoutTests/storage/filesystemaccess/resources/sync-access-handle-close.js 2021-10-22 16:52:26 UTC (rev 284692)
@@ -4,7 +4,15 @@
description("This test checks close() of FileSystemSyncAccessHandle");
-var accessHandle, promise;
+var accessHandle, fileHandle, error;
+const buffer = new ArrayBuffer(1);
+const options = { "at" : 0 };
+var functions = [
+ { name : "getSize" },
+ { name : "flush" },
+ { name : "read", args : [buffer, options], sync : true },
+ { name : "write", args : [buffer, options], sync : true },
+];
function finishTest(error)
{
@@ -14,31 +22,75 @@
finishJSTest();
}
-async function testFunctions()
+function testSyncFunction(currentFunction)
{
- shouldThrow("await accessHandle.close()");
- shouldThrow("await accessHandle.getSize()");
- shouldThrow("await accessHandle.flush()");
- shouldThrow("await accessHandle.read(new ArrayBuffer(1), { \"at\" : 0 })");
- shouldThrow("await accessHandle.write(new ArrayBuffer(1), { \"at\" : 0 })");
+ try {
+ var result = accessHandle[currentFunction.name].apply(accessHandle, currentFunction.args);
+ return null;
+ } catch (err) {
+ return err;
+ }
}
-async function test() {
+async function testAsyncFunction(func)
+{
+ var promise = accessHandle[func.name].apply(accessHandle, func.args);
+ return promise.then((value) => {
+ return func.name + " function should throw exception but didn't";
+ }, (err) => {
+ return err;
+ });
+}
+
+async function testFunctions()
+{
+ for (const func of functions) {
+ debug("testing " + func.name);
+
+ if (func.sync) {
+ error = testSyncFunction(func);
+ } else {
+ error = await testAsyncFunction(func);
+ }
+
+ shouldBeEqualToString("error.toString()", "InvalidStateError: AccessHandle is closing or closed");
+ }
+}
+
+async function testMultipleHandles()
+{
+ // Current limit of file descriptor count is 256.
+ for (let i = 0; i < 512; i++) {
+ try {
+ accessHandle = await fileHandle.createSyncAccessHandle();
+ await accessHandle.close();
+ } catch (err) {
+ throw "Failed at No." + i + " handle: " + err.toString();
+ }
+ }
+ debug("Create and close access handles successfully");
+}
+
+async function test()
+{
try {
var rootHandle = await navigator.storage.getDirectory();
// Create a new file for this test.
await rootHandle.removeEntry("sync-access-handle-close.txt").then(() => { }, () => { });
- var fileHandle = await rootHandle.getFileHandle("sync-access-handle-close.txt", { "create" : true });
+ fileHandle = await rootHandle.getFileHandle("sync-access-handle-close.txt", { "create" : true });
accessHandle = await fileHandle.createSyncAccessHandle();
var closePromise = accessHandle.close();
debug("test after invoking close():");
- testFunctions();
+ await testFunctions();
debug("test after close() is done:");
await closePromise;
- testFunctions();
+ await testFunctions();
+ debug("test closing multiple handles:");
+ await testMultipleHandles();
+
finishTest();
} catch (error) {
finishTest(error);
Modified: trunk/LayoutTests/storage/filesystemaccess/sync-access-handle-close-worker-expected.txt (284691 => 284692)
--- trunk/LayoutTests/storage/filesystemaccess/sync-access-handle-close-worker-expected.txt 2021-10-22 16:47:52 UTC (rev 284691)
+++ trunk/LayoutTests/storage/filesystemaccess/sync-access-handle-close-worker-expected.txt 2021-10-22 16:52:26 UTC (rev 284692)
@@ -5,17 +5,25 @@
Starting worker: resources/sync-access-handle-close.js
[Worker] test after invoking close():
-PASS [Worker] await accessHandle.close() threw exception SyntaxError: Unexpected identifier 'accessHandle'.
-PASS [Worker] await accessHandle.getSize() threw exception SyntaxError: Unexpected identifier 'accessHandle'.
-PASS [Worker] await accessHandle.flush() threw exception SyntaxError: Unexpected identifier 'accessHandle'.
-PASS [Worker] await accessHandle.read(new ArrayBuffer(1), { "at" : 0 }) threw exception SyntaxError: Unexpected identifier 'accessHandle'.
-PASS [Worker] await accessHandle.write(new ArrayBuffer(1), { "at" : 0 }) threw exception SyntaxError: Unexpected identifier 'accessHandle'.
+[Worker] testing getSize
+PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
+[Worker] testing flush
+PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
+[Worker] testing read
+PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
+[Worker] testing write
+PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
[Worker] test after close() is done:
-PASS [Worker] await accessHandle.close() threw exception SyntaxError: Unexpected identifier 'accessHandle'.
-PASS [Worker] await accessHandle.getSize() threw exception SyntaxError: Unexpected identifier 'accessHandle'.
-PASS [Worker] await accessHandle.flush() threw exception SyntaxError: Unexpected identifier 'accessHandle'.
-PASS [Worker] await accessHandle.read(new ArrayBuffer(1), { "at" : 0 }) threw exception SyntaxError: Unexpected identifier 'accessHandle'.
-PASS [Worker] await accessHandle.write(new ArrayBuffer(1), { "at" : 0 }) threw exception SyntaxError: Unexpected identifier 'accessHandle'.
+[Worker] testing getSize
+PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
+[Worker] testing flush
+PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
+[Worker] testing read
+PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
+[Worker] testing write
+PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
+[Worker] test closing multiple handles:
+[Worker] Create and close access handles successfully
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/Source/WebCore/ChangeLog (284691 => 284692)
--- trunk/Source/WebCore/ChangeLog 2021-10-22 16:47:52 UTC (rev 284691)
+++ trunk/Source/WebCore/ChangeLog 2021-10-22 16:52:26 UTC (rev 284692)
@@ -1,3 +1,19 @@
+2021-10-22 Sihui Liu <sihui_...@apple.com>
+
+ Followup to r284652: ensure file handle is closed in web process
+ https://bugs.webkit.org/show_bug.cgi?id=232127
+
+ Reviewed by Youenn Fablet.
+
+ Covered by test: storage/filesystemaccess/sync-access-handle-close-worker.html
+
+ * Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp:
+ (WebCore::FileSystemSyncAccessHandle::~FileSystemSyncAccessHandle): make sure file handle is closed when
+ FileSystemSyncAccessHandle is destroyed.
+ (WebCore::FileSystemSyncAccessHandle::closeInternal):
+ (WebCore::FileSystemSyncAccessHandle::close):
+ * Modules/filesystemaccess/FileSystemSyncAccessHandle.h:
+
2021-10-22 Ayumi Kojima <ayumi_koj...@apple.com>
Unreviewed, reverting r284606.
Modified: trunk/Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp (284691 => 284692)
--- trunk/Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp 2021-10-22 16:47:52 UTC (rev 284691)
+++ trunk/Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp 2021-10-22 16:52:26 UTC (rev 284692)
@@ -52,7 +52,7 @@
return;
ASSERT(m_closePromises.isEmpty());
- m_source->close(m_identifier, [](auto) { });
+ closeInternal([](auto) { });
}
bool FileSystemSyncAccessHandle::isClosingOrClosed() const
@@ -60,6 +60,12 @@
return m_closeResult || !m_closePromises.isEmpty();
}
+void FileSystemSyncAccessHandle::closeInternal(CompletionHandler<void(ExceptionOr<void>&&)>&& completionHandler)
+{
+ FileSystem::closeFile(m_file);
+ m_source->close(m_identifier, WTFMove(completionHandler));
+}
+
void FileSystemSyncAccessHandle::truncate(unsigned long long size, DOMPromiseDeferred<void>&& promise)
{
if (isClosingOrClosed())
@@ -112,11 +118,8 @@
if (isClosing)
return;
- FileSystem::closeFile(m_file);
- m_file = FileSystem::invalidPlatformFileHandle;
-
m_pendingOperationCount++;
- m_source->close(m_identifier, [this, protectedThis = Ref { *this }](auto result) mutable {
+ closeInternal([this, protectedThis = Ref { *this }](auto result) mutable {
m_pendingOperationCount--;
didClose(WTFMove(result));
});
Modified: trunk/Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.h (284691 => 284692)
--- trunk/Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.h 2021-10-22 16:47:52 UTC (rev 284691)
+++ trunk/Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.h 2021-10-22 16:52:26 UTC (rev 284692)
@@ -57,6 +57,7 @@
private:
FileSystemSyncAccessHandle(FileSystemFileHandle&, FileSystemSyncAccessHandleIdentifier, FileSystem::PlatformFileHandle);
bool isClosingOrClosed() const;
+ void closeInternal(CompletionHandler<void(ExceptionOr<void>&&)>&&);
Ref<FileSystemFileHandle> m_source;
FileSystemSyncAccessHandleIdentifier m_identifier;
Modified: trunk/Source/WebKit/ChangeLog (284691 => 284692)
--- trunk/Source/WebKit/ChangeLog 2021-10-22 16:47:52 UTC (rev 284691)
+++ trunk/Source/WebKit/ChangeLog 2021-10-22 16:52:26 UTC (rev 284692)
@@ -1,3 +1,18 @@
+2021-10-22 Sihui Liu <sihui_...@apple.com>
+
+ Followup to r284652: ensure file handle is closed in web process
+ https://bugs.webkit.org/show_bug.cgi?id=232127
+
+ Reviewed by Youenn Fablet.
+
+ * NetworkProcess/storage/FileSystemStorageHandle.cpp:
+ (WebKit::FileSystemStorageHandle::~FileSystemStorageHandle):
+ (WebKit::FileSystemStorageHandle::createSyncAccessHandle):
+ (WebKit::FileSystemStorageHandle::close):
+ * NetworkProcess/storage/FileSystemStorageHandle.h:
+ * Platform/IPC/cocoa/SharedFileHandleCocoa.cpp: an extra fd is created here and does not get closed.
+ (IPC::SharedFileHandle::decode):
+
2021-10-22 Youenn Fablet <you...@apple.com>
REGRESSION (Safari 15 - iOS15): [WebRTC] Increased audio latency while playing webrtc audio stream over audio element
Modified: trunk/Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp (284691 => 284692)
--- trunk/Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp 2021-10-22 16:47:52 UTC (rev 284691)
+++ trunk/Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp 2021-10-22 16:52:26 UTC (rev 284692)
@@ -63,6 +63,12 @@
}
}
+FileSystemStorageHandle::~FileSystemStorageHandle()
+{
+ if (m_handle != FileSystem::invalidPlatformFileHandle)
+ FileSystem::closeFile(m_handle);
+}
+
bool FileSystemStorageHandle::isSameEntry(WebCore::FileSystemHandleIdentifier identifier)
{
auto path = m_manager->getPath(identifier);
@@ -165,7 +171,6 @@
auto ipcHandle = IPC::SharedFileHandle::create(m_handle);
if (!ipcHandle) {
FileSystem::closeFile(m_handle);
- m_handle = FileSystem::invalidPlatformFileHandle;
return makeUnexpected(FileSystemStorageError::BackendNotSupported);
}
@@ -223,16 +228,15 @@
std::optional<FileSystemStorageError> FileSystemStorageHandle::close(WebCore::FileSystemSyncAccessHandleIdentifier accessHandleIdentifier)
{
- if (!m_manager)
- return FileSystemStorageError::Unknown;
-
if (!m_activeSyncAccessHandle || *m_activeSyncAccessHandle != accessHandleIdentifier)
return FileSystemStorageError::Unknown;
ASSERT(m_handle != FileSystem::invalidPlatformFileHandle);
FileSystem::closeFile(m_handle);
- m_handle = FileSystem::invalidPlatformFileHandle;
+ if (!m_manager)
+ return FileSystemStorageError::Unknown;
+
m_manager->releaseLockForFile(m_path, m_identifier);
m_activeSyncAccessHandle = std::nullopt;
Modified: trunk/Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.h (284691 => 284692)
--- trunk/Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.h 2021-10-22 16:47:52 UTC (rev 284691)
+++ trunk/Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.h 2021-10-22 16:52:26 UTC (rev 284692)
@@ -44,6 +44,7 @@
public:
enum class Type : uint8_t { File, Directory, Any };
FileSystemStorageHandle(FileSystemStorageManager&, Type, String&& path, String&& name);
+ ~FileSystemStorageHandle();
WebCore::FileSystemHandleIdentifier identifier() const { return m_identifier; }
const String& path() const { return m_path; }
Modified: trunk/Source/WebKit/Platform/IPC/cocoa/SharedFileHandleCocoa.cpp (284691 => 284692)
--- trunk/Source/WebKit/Platform/IPC/cocoa/SharedFileHandleCocoa.cpp 2021-10-22 16:47:52 UTC (rev 284691)
+++ trunk/Source/WebKit/Platform/IPC/cocoa/SharedFileHandleCocoa.cpp 2021-10-22 16:52:26 UTC (rev 284692)
@@ -57,7 +57,7 @@
if (fd == -1)
return SharedFileHandle { };
- return SharedFileHandle::create(fileport_makefd(machPort.port()));
+ return SharedFileHandle::create(fd);
}
} // namespace IPC
