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

Changeset 284815 in webkit


Ignore:
Timestamp:
Oct 25, 2021, 12:10:30 PM (5 years ago)
Author:
Alan Coon
Message:

Cherry-pick r284692. rdar://problem/84553142

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:

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@284692 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-612-branch
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-612-branch/LayoutTests/ChangeLog

    r284814 r284815  
     12021-10-25  Null  <null@apple.com>
     2
     3        Cherry-pick r284692. rdar://problem/84553142
     4
     5    Followup to r284652: ensure file handle is closed in web process
     6    https://bugs.webkit.org/show_bug.cgi?id=232127
     7   
     8    Reviewed by Youenn Fablet.
     9   
     10    Source/WebCore:
     11   
     12    Covered by test: storage/filesystemaccess/sync-access-handle-close-worker.html
     13   
     14    * Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp:
     15    (WebCore::FileSystemSyncAccessHandle::~FileSystemSyncAccessHandle): make sure file handle is closed when
     16    FileSystemSyncAccessHandle is destroyed.
     17    (WebCore::FileSystemSyncAccessHandle::closeInternal):
     18    (WebCore::FileSystemSyncAccessHandle::close):
     19    * Modules/filesystemaccess/FileSystemSyncAccessHandle.h:
     20   
     21    Source/WebKit:
     22   
     23    * NetworkProcess/storage/FileSystemStorageHandle.cpp:
     24    (WebKit::FileSystemStorageHandle::~FileSystemStorageHandle):
     25    (WebKit::FileSystemStorageHandle::createSyncAccessHandle):
     26    (WebKit::FileSystemStorageHandle::close):
     27    * NetworkProcess/storage/FileSystemStorageHandle.h:
     28    * Platform/IPC/cocoa/SharedFileHandleCocoa.cpp: an extra fd is created here and does not get closed.
     29    (IPC::SharedFileHandle::decode):
     30   
     31    LayoutTests:
     32   
     33    * storage/filesystemaccess/resources/sync-access-handle-close.js:
     34    (testSyncFunction):
     35    (async testAsyncFunction):
     36    (async testFunctions):
     37    (async testMultipleHandles):
     38    (async test):
     39    * storage/filesystemaccess/sync-access-handle-close-worker-expected.txt:
     40   
     41   
     42    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284692 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     43
     44    2021-10-22  Sihui Liu  <sihui_liu@apple.com>
     45
     46            Followup to r284652: ensure file handle is closed in web process
     47            https://bugs.webkit.org/show_bug.cgi?id=232127
     48
     49            Reviewed by Youenn Fablet.
     50
     51            * storage/filesystemaccess/resources/sync-access-handle-close.js:
     52            (testSyncFunction):
     53            (async testAsyncFunction):
     54            (async testFunctions):
     55            (async testMultipleHandles):
     56            (async test):
     57            * storage/filesystemaccess/sync-access-handle-close-worker-expected.txt:
     58
    1592021-10-25  Null  <null@apple.com>
    260

  • branches/safari-612-branch/LayoutTests/storage/filesystemaccess/resources/sync-access-handle-close.js

    r284814 r284815  
    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();

  • branches/safari-612-branch/LayoutTests/storage/filesystemaccess/sync-access-handle-close-worker-expected.txt

    r284814 r284815  
    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

  • branches/safari-612-branch/Source/WebCore/ChangeLog

    r284814 r284815  
     12021-10-25  Null  <null@apple.com>
     2
     3        Cherry-pick r284692. rdar://problem/84553142
     4
     5    Followup to r284652: ensure file handle is closed in web process
     6    https://bugs.webkit.org/show_bug.cgi?id=232127
     7   
     8    Reviewed by Youenn Fablet.
     9   
     10    Source/WebCore:
     11   
     12    Covered by test: storage/filesystemaccess/sync-access-handle-close-worker.html
     13   
     14    * Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp:
     15    (WebCore::FileSystemSyncAccessHandle::~FileSystemSyncAccessHandle): make sure file handle is closed when
     16    FileSystemSyncAccessHandle is destroyed.
     17    (WebCore::FileSystemSyncAccessHandle::closeInternal):
     18    (WebCore::FileSystemSyncAccessHandle::close):
     19    * Modules/filesystemaccess/FileSystemSyncAccessHandle.h:
     20   
     21    Source/WebKit:
     22   
     23    * NetworkProcess/storage/FileSystemStorageHandle.cpp:
     24    (WebKit::FileSystemStorageHandle::~FileSystemStorageHandle):
     25    (WebKit::FileSystemStorageHandle::createSyncAccessHandle):
     26    (WebKit::FileSystemStorageHandle::close):
     27    * NetworkProcess/storage/FileSystemStorageHandle.h:
     28    * Platform/IPC/cocoa/SharedFileHandleCocoa.cpp: an extra fd is created here and does not get closed.
     29    (IPC::SharedFileHandle::decode):
     30   
     31    LayoutTests:
     32   
     33    * storage/filesystemaccess/resources/sync-access-handle-close.js:
     34    (testSyncFunction):
     35    (async testAsyncFunction):
     36    (async testFunctions):
     37    (async testMultipleHandles):
     38    (async test):
     39    * storage/filesystemaccess/sync-access-handle-close-worker-expected.txt:
     40   
     41   
     42    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284692 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     43
     44    2021-10-22  Sihui Liu  <sihui_liu@apple.com>
     45
     46            Followup to r284652: ensure file handle is closed in web process
     47            https://bugs.webkit.org/show_bug.cgi?id=232127
     48
     49            Reviewed by Youenn Fablet.
     50
     51            Covered by test: storage/filesystemaccess/sync-access-handle-close-worker.html
     52
     53            * Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp:
     54            (WebCore::FileSystemSyncAccessHandle::~FileSystemSyncAccessHandle): make sure file handle is closed when
     55            FileSystemSyncAccessHandle is destroyed.
     56            (WebCore::FileSystemSyncAccessHandle::closeInternal):
     57            (WebCore::FileSystemSyncAccessHandle::close):
     58            * Modules/filesystemaccess/FileSystemSyncAccessHandle.h:
     59
    1602021-10-25  Null  <null@apple.com>
    261

  • branches/safari-612-branch/Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp

    r284814 r284815  
    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));

  • branches/safari-612-branch/Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.h

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

  • branches/safari-612-branch/Source/WebKit/ChangeLog

    r284813 r284815  
     12021-10-25  Null  <null@apple.com>
     2
     3        Cherry-pick r284692. rdar://problem/84553142
     4
     5    Followup to r284652: ensure file handle is closed in web process
     6    https://bugs.webkit.org/show_bug.cgi?id=232127
     7   
     8    Reviewed by Youenn Fablet.
     9   
     10    Source/WebCore:
     11   
     12    Covered by test: storage/filesystemaccess/sync-access-handle-close-worker.html
     13   
     14    * Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp:
     15    (WebCore::FileSystemSyncAccessHandle::~FileSystemSyncAccessHandle): make sure file handle is closed when
     16    FileSystemSyncAccessHandle is destroyed.
     17    (WebCore::FileSystemSyncAccessHandle::closeInternal):
     18    (WebCore::FileSystemSyncAccessHandle::close):
     19    * Modules/filesystemaccess/FileSystemSyncAccessHandle.h:
     20   
     21    Source/WebKit:
     22   
     23    * NetworkProcess/storage/FileSystemStorageHandle.cpp:
     24    (WebKit::FileSystemStorageHandle::~FileSystemStorageHandle):
     25    (WebKit::FileSystemStorageHandle::createSyncAccessHandle):
     26    (WebKit::FileSystemStorageHandle::close):
     27    * NetworkProcess/storage/FileSystemStorageHandle.h:
     28    * Platform/IPC/cocoa/SharedFileHandleCocoa.cpp: an extra fd is created here and does not get closed.
     29    (IPC::SharedFileHandle::decode):
     30   
     31    LayoutTests:
     32   
     33    * storage/filesystemaccess/resources/sync-access-handle-close.js:
     34    (testSyncFunction):
     35    (async testAsyncFunction):
     36    (async testFunctions):
     37    (async testMultipleHandles):
     38    (async test):
     39    * storage/filesystemaccess/sync-access-handle-close-worker-expected.txt:
     40   
     41   
     42    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284692 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     43
     44    2021-10-22  Sihui Liu  <sihui_liu@apple.com>
     45
     46            Followup to r284652: ensure file handle is closed in web process
     47            https://bugs.webkit.org/show_bug.cgi?id=232127
     48
     49            Reviewed by Youenn Fablet.
     50
     51            * NetworkProcess/storage/FileSystemStorageHandle.cpp:
     52            (WebKit::FileSystemStorageHandle::~FileSystemStorageHandle):
     53            (WebKit::FileSystemStorageHandle::createSyncAccessHandle):
     54            (WebKit::FileSystemStorageHandle::close):
     55            * NetworkProcess/storage/FileSystemStorageHandle.h:
     56            * Platform/IPC/cocoa/SharedFileHandleCocoa.cpp: an extra fd is created here and does not get closed.
     57            (IPC::SharedFileHandle::decode):
     58
    1592021-10-25  Null  <null@apple.com>
    260

  • branches/safari-612-branch/Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp

    r284425 r284815  
    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);

  • branches/safari-612-branch/Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.h

    r284425 r284815  
    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; }

  • branches/safari-612-branch/Source/WebKit/Platform/IPC/cocoa/SharedFileHandleCocoa.cpp

    r284424 r284815  
    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.