⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 284692 in webkit


Ignore:
Timestamp:
Oct 22, 2021, 9:52:26 AM (5 years ago)
Author:
sihui_liu@apple.com
Message:

Followup to r284652: ensure file handle is closed in web process
​https://bugs.webkit.org/show_bug.cgi?id=232127

Reviewed by Youenn Fablet.

Source/WebCore:

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:

Source/WebKit:

  • 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):

LayoutTests:

  • 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:
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r284686 r284692  
     12021-10-22  Sihui Liu  <sihui_liu@apple.com>
     2
     3        Followup to r284652: ensure file handle is closed in web process
     4        https://bugs.webkit.org/show_bug.cgi?id=232127
     5
     6        Reviewed by Youenn Fablet.
     7
     8        * storage/filesystemaccess/resources/sync-access-handle-close.js:
     9        (testSyncFunction):
     10        (async testAsyncFunction):
     11        (async testFunctions):
     12        (async testMultipleHandles):
     13        (async test):
     14        * storage/filesystemaccess/sync-access-handle-close-worker-expected.txt:
     15
    1162021-10-22  Chris Dumez  <cdumez@apple.com>
    217

  • trunk/LayoutTests/storage/filesystemaccess/resources/sync-access-handle-close.js

    r284652 r284692  
    55description("This test checks close() of FileSystemSyncAccessHandle");
    66
    7 var accessHandle, promise;
     7var accessHandle, fileHandle, error;
     8const buffer = new ArrayBuffer(1);
     9const options = { "at" : 0 };
     10var functions = [
     11    { name : "getSize" },
     12    { name : "flush" },
     13    { name : "read", args : [buffer, options], sync : true },
     14    { name : "write", args : [buffer, options], sync : true },
     15];
    816
    917function finishTest(error)
    … …  
    1523}
    1624
    17 async function testFunctions()
     25function testSyncFunction(currentFunction)
    1826{
    19     shouldThrow("await accessHandle.close()");
    20     shouldThrow("await accessHandle.getSize()");
    21     shouldThrow("await accessHandle.flush()");
    22     shouldThrow("await accessHandle.read(new ArrayBuffer(1), { \"at\" : 0 })");
    23     shouldThrow("await accessHandle.write(new ArrayBuffer(1), { \"at\" : 0 })");
     27    try {
     28        var result = accessHandle[currentFunction.name].apply(accessHandle, currentFunction.args);
     29        return null;
     30    } catch (err) {
     31        return err;
     32    }
    2433}
    2534
    26 async function test() {
     35async function testAsyncFunction(func)
     36{
     37    var promise = accessHandle[func.name].apply(accessHandle, func.args);
     38    return promise.then((value) => {
     39        return func.name + " function should throw exception but didn't";
     40    }, (err) => {
     41        return err;
     42    });
     43}
     44
     45async function testFunctions()
     46{
     47    for (const func of functions) {
     48        debug("testing " + func.name);
     49
     50        if (func.sync) {
     51            error = testSyncFunction(func);
     52        } else {
     53            error = await testAsyncFunction(func);
     54        }
     55
     56        shouldBeEqualToString("error.toString()", "InvalidStateError: AccessHandle is closing or closed");
     57    }
     58}
     59
     60async function testMultipleHandles()
     61{
     62    // Current limit of file descriptor count is 256.
     63    for (let i = 0; i < 512; i++) {
     64        try {
     65            accessHandle = await fileHandle.createSyncAccessHandle();
     66            await accessHandle.close();
     67        } catch (err) {
     68            throw "Failed at No." + i + " handle: " + err.toString();
     69        }
     70    }
     71    debug("Create and close access handles successfully");
     72}
     73
     74async function test()
     75{
    2776    try {
    2877        var rootHandle = await navigator.storage.getDirectory();
    2978        // Create a new file for this test.
    3079        await rootHandle.removeEntry("sync-access-handle-close.txt").then(() => { }, () => { });
    31         var fileHandle = await rootHandle.getFileHandle("sync-access-handle-close.txt", { "create" : true  });
     80        fileHandle = await rootHandle.getFileHandle("sync-access-handle-close.txt", { "create" : true  });
    3281        accessHandle = await fileHandle.createSyncAccessHandle();
    3382
    3483        var closePromise = accessHandle.close();
    3584        debug("test after invoking close():");
    36         testFunctions();
     85        await testFunctions();
    3786
    3887        debug("test after close() is done:");
    3988        await closePromise;
    40         testFunctions();
     89        await testFunctions();
     90
     91        debug("test closing multiple handles:");
     92        await testMultipleHandles();
    4193
    4294        finishTest();

  • trunk/LayoutTests/storage/filesystemaccess/sync-access-handle-close-worker-expected.txt

    r284652 r284692  
    66Starting worker: resources/sync-access-handle-close.js
    77[Worker] test after invoking close():
    8 PASS [Worker] await accessHandle.close() threw exception SyntaxError: Unexpected identifier 'accessHandle'.
    9 PASS [Worker] await accessHandle.getSize() threw exception SyntaxError: Unexpected identifier 'accessHandle'.
    10 PASS [Worker] await accessHandle.flush() threw exception SyntaxError: Unexpected identifier 'accessHandle'.
    11 PASS [Worker] await accessHandle.read(new ArrayBuffer(1), { "at" : 0 }) threw exception SyntaxError: Unexpected identifier 'accessHandle'.
    12 PASS [Worker] await accessHandle.write(new ArrayBuffer(1), { "at" : 0 }) threw exception SyntaxError: Unexpected identifier 'accessHandle'.
     8[Worker] testing getSize
     9PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
     10[Worker] testing flush
     11PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
     12[Worker] testing read
     13PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
     14[Worker] testing write
     15PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
    1316[Worker] test after close() is done:
    14 PASS [Worker] await accessHandle.close() threw exception SyntaxError: Unexpected identifier 'accessHandle'.
    15 PASS [Worker] await accessHandle.getSize() threw exception SyntaxError: Unexpected identifier 'accessHandle'.
    16 PASS [Worker] await accessHandle.flush() threw exception SyntaxError: Unexpected identifier 'accessHandle'.
    17 PASS [Worker] await accessHandle.read(new ArrayBuffer(1), { "at" : 0 }) threw exception SyntaxError: Unexpected identifier 'accessHandle'.
    18 PASS [Worker] await accessHandle.write(new ArrayBuffer(1), { "at" : 0 }) threw exception SyntaxError: Unexpected identifier 'accessHandle'.
     17[Worker] testing getSize
     18PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
     19[Worker] testing flush
     20PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
     21[Worker] testing read
     22PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
     23[Worker] testing write
     24PASS [Worker] error.toString() is "InvalidStateError: AccessHandle is closing or closed"
     25[Worker] test closing multiple handles:
     26[Worker] Create and close access handles successfully
    1927PASS successfullyParsed is true
    2028

  • trunk/Source/WebCore/ChangeLog

    r284687 r284692  
     12021-10-22  Sihui Liu  <sihui_liu@apple.com>
     2
     3        Followup to r284652: ensure file handle is closed in web process
     4        https://bugs.webkit.org/show_bug.cgi?id=232127
     5
     6        Reviewed by Youenn Fablet.
     7
     8        Covered by test: storage/filesystemaccess/sync-access-handle-close-worker.html 
     9
     10        * Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp:
     11        (WebCore::FileSystemSyncAccessHandle::~FileSystemSyncAccessHandle): make sure file handle is closed when
     12        FileSystemSyncAccessHandle is destroyed.
     13        (WebCore::FileSystemSyncAccessHandle::closeInternal):
     14        (WebCore::FileSystemSyncAccessHandle::close):
     15        * Modules/filesystemaccess/FileSystemSyncAccessHandle.h:
     16
    1172021-10-22  Ayumi Kojima  <ayumi_kojima@apple.com>
    218

  • trunk/Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp

    r284652 r284692  
    5353
    5454    ASSERT(m_closePromises.isEmpty());
    55     m_source->close(m_identifier, [](auto) { });
     55    closeInternal([](auto) { });
    5656}
    5757
    … …  
    5959{
    6060    return m_closeResult || !m_closePromises.isEmpty();
     61}
     62
     63void FileSystemSyncAccessHandle::closeInternal(CompletionHandler<void(ExceptionOr<void>&&)>&& completionHandler)
     64{
     65    FileSystem::closeFile(m_file);
     66    m_source->close(m_identifier, WTFMove(completionHandler));
    6167}
    6268
    … …  
    113119        return;
    114120
    115     FileSystem::closeFile(m_file);
    116     m_file = FileSystem::invalidPlatformFileHandle;
    117 
    118121    m_pendingOperationCount++;
    119     m_source->close(m_identifier, [this, protectedThis = Ref { *this }](auto result) mutable {
     122    closeInternal([this, protectedThis = Ref { *this }](auto result) mutable {
    120123        m_pendingOperationCount--;
    121124        didClose(WTFMove(result));

  • trunk/Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.h

    r284652 r284692  
    5858    FileSystemSyncAccessHandle(FileSystemFileHandle&, FileSystemSyncAccessHandleIdentifier, FileSystem::PlatformFileHandle);
    5959    bool isClosingOrClosed() const;
     60    void closeInternal(CompletionHandler<void(ExceptionOr<void>&&)>&&);
    6061
    6162    Ref<FileSystemFileHandle> m_source;

  • trunk/Source/WebKit/ChangeLog

    r284689 r284692  
     12021-10-22  Sihui Liu  <sihui_liu@apple.com>
     2
     3        Followup to r284652: ensure file handle is closed in web process
     4        https://bugs.webkit.org/show_bug.cgi?id=232127
     5
     6        Reviewed by Youenn Fablet.
     7
     8        * NetworkProcess/storage/FileSystemStorageHandle.cpp:
     9        (WebKit::FileSystemStorageHandle::~FileSystemStorageHandle):
     10        (WebKit::FileSystemStorageHandle::createSyncAccessHandle):
     11        (WebKit::FileSystemStorageHandle::close):
     12        * NetworkProcess/storage/FileSystemStorageHandle.h:
     13        * Platform/IPC/cocoa/SharedFileHandleCocoa.cpp: an extra fd is created here and does not get closed.
     14        (IPC::SharedFileHandle::decode):
     15
    1162021-10-22  Youenn Fablet  <youenn@apple.com>
    217

  • trunk/Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp

    r284142 r284692  
    6464}
    6565
     66FileSystemStorageHandle::~FileSystemStorageHandle()
     67{
     68    if (m_handle != FileSystem::invalidPlatformFileHandle)
     69        FileSystem::closeFile(m_handle);
     70}
     71
    6672bool FileSystemStorageHandle::isSameEntry(WebCore::FileSystemHandleIdentifier identifier)
    6773{
    … …  
    166172    if (!ipcHandle) {
    167173        FileSystem::closeFile(m_handle);
    168         m_handle = FileSystem::invalidPlatformFileHandle;
    169174        return makeUnexpected(FileSystemStorageError::BackendNotSupported);
    170175    }
    … …  
    224229std::optional<FileSystemStorageError> FileSystemStorageHandle::close(WebCore::FileSystemSyncAccessHandleIdentifier accessHandleIdentifier)
    225230{
    226     if (!m_manager)
    227         return FileSystemStorageError::Unknown;
    228 
    229231    if (!m_activeSyncAccessHandle || *m_activeSyncAccessHandle != accessHandleIdentifier)
    230232        return FileSystemStorageError::Unknown;
    … …  
    232234    ASSERT(m_handle != FileSystem::invalidPlatformFileHandle);
    233235    FileSystem::closeFile(m_handle);
    234     m_handle = FileSystem::invalidPlatformFileHandle;
     236
     237    if (!m_manager)
     238        return FileSystemStorageError::Unknown;
    235239
    236240    m_manager->releaseLockForFile(m_path, m_identifier);

  • trunk/Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.h

    r284124 r284692  
    4545    enum class Type : uint8_t { File, Directory, Any };
    4646    FileSystemStorageHandle(FileSystemStorageManager&, Type, String&& path, String&& name);
     47    ~FileSystemStorageHandle();
    4748
    4849    WebCore::FileSystemHandleIdentifier identifier() const { return m_identifier; }

  • trunk/Source/WebKit/Platform/IPC/cocoa/SharedFileHandleCocoa.cpp

    r284059 r284692  
    5858        return SharedFileHandle { };
    5959
    60     return SharedFileHandle::create(fileport_makefd(machPort.port()));
     60    return SharedFileHandle::create(fd);
    6161}
    6262
Note: See TracChangeset for help on using the changeset viewer.