Skip to content

Commit

Permalink
[WK2][macOS] Update sandbox profile caching to not require file locking
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=258940
rdar://111383802

Reviewed by Ben Nham, Per Arne Vollan and Brent Fulgham.

As a process launch optimization on macOS, we cache the compiled sandbox
profile to disk and reuse this cached profile for follow-up process
launches.

The logic was relying to using file locking to avoid reading partially
written sandbox profiles, which would lead to issues if the process
suspends while in the middle of doing this work.

To avoid locking, we now write the compiled sandbox profile to a
temporary path which includes the current process's PID, before moving
it to the final destination (using rename()). Because rename() is atomic,
this should achieve what we want without file locking.

* Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:
(WebKit::writeSandboxDataToCacheFile):
(WebKit::tryApplyCachedSandbox):

Canonical link: https://commits.webkit.org/265823@main
  • Loading branch information
cdumez committed Jul 6, 2023
1 parent 46f465b commit 9ec2b57
Showing 1 changed file with 14 additions and 3 deletions.
17 changes: 14 additions & 3 deletions Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,19 @@ static bool ensureSandboxCacheDirectory(const SandboxInfo& info)

static bool writeSandboxDataToCacheFile(const SandboxInfo& info, const Vector<char>& cacheFile)
{
FileHandle file { info.filePath, FileSystem::FileOpenMode::Truncate, FileSystem::FileLockMode::Exclusive };
return file.write(cacheFile.data(), cacheFile.size()) == safeCast<int>(cacheFile.size());
// To avoid locking, write the sandbox data to a temporary path including the current process' PID
// then rename it to the final cache path.
auto temporaryPath = makeString(info.filePath, '-', getpid());
FileHandle file { temporaryPath, FileSystem::FileOpenMode::Truncate };
if (file.write(cacheFile.data(), cacheFile.size()) != safeCast<int>(cacheFile.size())) {
FileSystem::deleteFile(temporaryPath);
return false;
}
if (!FileSystem::moveFile(temporaryPath, info.filePath)) {
FileSystem::deleteFile(temporaryPath);
return false;
}
return true;
}

static SandboxProfilePtr compileAndCacheSandboxProfile(const SandboxInfo& info)
Expand Down Expand Up @@ -458,7 +469,7 @@ static bool tryApplyCachedSandbox(const SandboxInfo& info)
return false;
#endif

auto contents = fileContents(info.filePath, true, FileSystem::FileLockMode::Shared);
auto contents = fileContents(info.filePath);
if (!contents || contents->isEmpty())
return false;
Vector<char> cachedSandboxContents = WTFMove(*contents);
Expand Down

0 comments on commit 9ec2b57

Please sign in to comment.