|
@@ -0,0 +1,765 @@
|
|
|
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
+From: Victor Costan <[email protected]>
|
|
|
+Date: Thu, 11 Nov 2021 00:22:30 +0000
|
|
|
+Subject: M96: Storage Foundation: Share FileState ownership with I/O threads.
|
|
|
+
|
|
|
+blink::NativeIOFile methods implementing the Storage Foundation
|
|
|
+JavaScript API pass raw pointers to NativeIOFile::FileState instances to
|
|
|
+their corresponding blink::NativeIOFile::Do*() methods, which rely on
|
|
|
+that CrossThreadPersistent<NativeIOFile> arguments to keep the
|
|
|
+underlying NativeIOFile::FileState instances alive.
|
|
|
+
|
|
|
+CrossThreadPersistent can be used across threads to keep a garbage
|
|
|
+collected object alive, together with any non-garbage-collected objects
|
|
|
+that it owns. However, relying on CrossThreadPersistent existence to
|
|
|
+access the owned objects on a different thread is not safe.
|
|
|
+cppgc::subtle::CrossThreadPersistent (blink::CrossThreadPersistent is an
|
|
|
+alias to that) has comments explaining that the garbage collected heap
|
|
|
+can go away while the CrossThreadPersistent instance exists.
|
|
|
+
|
|
|
+This CL fixes the problem by having the ownership of
|
|
|
+NativeIOFile::FileState be shared between the corresponding NativeIOFile
|
|
|
+instance and any threads doing I/O on the FileState.
|
|
|
+
|
|
|
+(cherry picked from commit 7dc02206707362f3f92cea93f8eb2fa4af0d375f)
|
|
|
+
|
|
|
+Bug: 1240593
|
|
|
+Change-Id: I5c9c818bcb23316fe1fd5afa57ed9c3fdb034377
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3269947
|
|
|
+Commit-Queue: Victor Costan <[email protected]>
|
|
|
+Reviewed-by: Austin Sullivan <[email protected]>
|
|
|
+Reviewed-by: Marijn Kruisselbrink <[email protected]>
|
|
|
+Reviewed-by: enne <[email protected]>
|
|
|
+Cr-Original-Commit-Position: refs/heads/main@{#940130}
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3272672
|
|
|
+Bot-Commit: Rubber Stamper <[email protected]>
|
|
|
+Cr-Commit-Position: refs/branch-heads/4664@{#945}
|
|
|
+Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512}
|
|
|
+
|
|
|
+diff --git a/third_party/blink/renderer/modules/native_io/native_io_file.cc b/third_party/blink/renderer/modules/native_io/native_io_file.cc
|
|
|
+index 4d5aa4efa13930aea4886bac0fd8ba892ce8b5a5..615c1d3a20cba732a3d981bf6b2df181e56d2727 100644
|
|
|
+--- a/third_party/blink/renderer/modules/native_io/native_io_file.cc
|
|
|
++++ b/third_party/blink/renderer/modules/native_io/native_io_file.cc
|
|
|
+@@ -9,6 +9,7 @@
|
|
|
+ #include "base/check.h"
|
|
|
+ #include "base/files/file.h"
|
|
|
+ #include "base/location.h"
|
|
|
++#include "base/memory/scoped_refptr.h"
|
|
|
+ #include "base/numerics/checked_math.h"
|
|
|
+ #include "base/sequenced_task_runner.h"
|
|
|
+ #include "base/task/thread_pool.h"
|
|
|
+@@ -47,31 +48,167 @@
|
|
|
+
|
|
|
+ namespace blink {
|
|
|
+
|
|
|
+-struct NativeIOFile::FileState {
|
|
|
+- explicit FileState(base::File file) : file(std::move(file)) {}
|
|
|
++// State and logic for performing file I/O off the JavaScript thread.
|
|
|
++//
|
|
|
++// Instances are allocated on the PartitionAlloc heap. Instances cannot be
|
|
|
++// garbage-collected, because garbage collected heaps get deallocated when the
|
|
|
++// underlying threads are terminated, and we need a guarantee that each
|
|
|
++// instance remains alive while it is used by a thread performing file I/O.
|
|
|
++//
|
|
|
++// Instances are initially constructed on a Blink thread that executes
|
|
|
++// JavaScript, which can be Blink's main thread, or a worker thread. Afterwards,
|
|
|
++// instances are (mostly*) only accessed on dedicated threads that do blocking
|
|
|
++// file I/O.
|
|
|
++//
|
|
|
++// Mostly*: On macOS < 10.15, SetLength() synchronously accesses FileState on
|
|
|
++// the JavaScript thread. This could be fixed with extra thread hopping. We're
|
|
|
++// not currently planning to invest in the fix.
|
|
|
++class NativeIOFile::FileState
|
|
|
++ : public base::RefCountedThreadSafe<NativeIOFile::FileState> {
|
|
|
++ public:
|
|
|
++ explicit FileState(base::File file) : file_(std::move(file)) {
|
|
|
++ DCHECK(file_.IsValid());
|
|
|
++ }
|
|
|
+
|
|
|
+ FileState(const FileState&) = delete;
|
|
|
+ FileState& operator=(const FileState&) = delete;
|
|
|
+
|
|
|
+ ~FileState() = default;
|
|
|
+
|
|
|
++ // Returns true until Close() is called. Returns false afterwards.
|
|
|
++ //
|
|
|
++ // On macOS < 10.15, returns false between a TakeFile() call and the
|
|
|
++ // corresponding SetFile() call.
|
|
|
++ bool IsValid() {
|
|
|
++ DCHECK(!IsMainThread());
|
|
|
++
|
|
|
++ WTF::MutexLocker locker(mutex_);
|
|
|
++ return file_.IsValid();
|
|
|
++ }
|
|
|
++
|
|
|
++ void Close() {
|
|
|
++ DCHECK(!IsMainThread());
|
|
|
++
|
|
|
++ WTF::MutexLocker locker(mutex_);
|
|
|
++ DCHECK(file_.IsValid()) << __func__ << " called on invalid file";
|
|
|
++
|
|
|
++ file_.Close();
|
|
|
++ }
|
|
|
++
|
|
|
++ // Returns {length, base::File::FILE_OK} in case of success.
|
|
|
++ // Returns {invalid number, error} in case of failure.
|
|
|
++ std::pair<int64_t, base::File::Error> GetLength() {
|
|
|
++ DCHECK(!IsMainThread());
|
|
|
++
|
|
|
++ WTF::MutexLocker mutex_locker(mutex_);
|
|
|
++ DCHECK(file_.IsValid()) << __func__ << " called on invalid file";
|
|
|
++
|
|
|
++ int64_t length = file_.GetLength();
|
|
|
++ base::File::Error error =
|
|
|
++ (length < 0) ? file_.GetLastFileError() : base::File::FILE_OK;
|
|
|
++
|
|
|
++ return {length, error};
|
|
|
++ }
|
|
|
++
|
|
|
++ // Returns {expected_length, base::File::FILE_OK} in case of success.
|
|
|
++ // Returns {actual file length, error} in case of failure.
|
|
|
++ std::pair<int64_t, base::File::Error> SetLength(int64_t expected_length) {
|
|
|
++ DCHECK(!IsMainThread());
|
|
|
++ DCHECK_GE(expected_length, 0);
|
|
|
++
|
|
|
++ WTF::MutexLocker mutex_locker(mutex_);
|
|
|
++ DCHECK(file_.IsValid()) << __func__ << " called on invalid file";
|
|
|
++
|
|
|
++ bool success = file_.SetLength(expected_length);
|
|
|
++ base::File::Error error =
|
|
|
++ success ? base::File::FILE_OK : file_.GetLastFileError();
|
|
|
++ int64_t actual_length = success ? expected_length : file_.GetLength();
|
|
|
++
|
|
|
++ return {actual_length, error};
|
|
|
++ }
|
|
|
++
|
|
|
++#if defined(OS_MAC)
|
|
|
++ // Used to implement browser-side SetLength() on macOS < 10.15.
|
|
|
++ base::File TakeFile() {
|
|
|
++ WTF::MutexLocker mutex_locker(mutex_);
|
|
|
++ DCHECK(file_.IsValid()) << __func__ << " called on invalid file";
|
|
|
++
|
|
|
++ return std::move(file_);
|
|
|
++ }
|
|
|
++
|
|
|
++ // Used to implement browser-side SetLength() on macOS < 10.15.
|
|
|
++ void SetFile(base::File file) {
|
|
|
++ WTF::MutexLocker locker(mutex_);
|
|
|
++ DCHECK(!file_.IsValid()) << __func__ << " called on valid file";
|
|
|
++
|
|
|
++ file_ = std::move(file);
|
|
|
++ }
|
|
|
++#endif // defined(OS_MAC)
|
|
|
++
|
|
|
++ // Returns {read byte count, base::File::FILE_OK} in case of success.
|
|
|
++ // Returns {invalid number, error} in case of failure.
|
|
|
++ std::pair<int, base::File::Error> Read(NativeIODataBuffer* buffer,
|
|
|
++ int64_t file_offset,
|
|
|
++ int read_size) {
|
|
|
++ DCHECK(!IsMainThread());
|
|
|
++ DCHECK(buffer);
|
|
|
++ DCHECK_GE(file_offset, 0);
|
|
|
++ DCHECK_GE(read_size, 0);
|
|
|
++
|
|
|
++ WTF::MutexLocker mutex_locker(mutex_);
|
|
|
++ DCHECK(file_.IsValid()) << __func__ << " called on invalid file";
|
|
|
++
|
|
|
++ int read_bytes = file_.Read(file_offset, buffer->Data(), read_size);
|
|
|
++ base::File::Error error =
|
|
|
++ (read_bytes < 0) ? file_.GetLastFileError() : base::File::FILE_OK;
|
|
|
++
|
|
|
++ return {read_bytes, error};
|
|
|
++ }
|
|
|
++
|
|
|
++ // Returns {0, write_size, base::File::FILE_OK} in case of success.
|
|
|
++ // Returns {actual file length, written bytes, base::File::OK} in case of a
|
|
|
++ // short write.
|
|
|
++ // Returns {actual file length, invalid number, error} in case of failure.
|
|
|
++ std::tuple<int64_t, int, base::File::Error> Write(NativeIODataBuffer* buffer,
|
|
|
++ int64_t file_offset,
|
|
|
++ int write_size) {
|
|
|
++ DCHECK(!IsMainThread());
|
|
|
++ DCHECK(buffer);
|
|
|
++ DCHECK_GE(file_offset, 0);
|
|
|
++ DCHECK_GE(write_size, 0);
|
|
|
++
|
|
|
++ WTF::MutexLocker mutex_locker(mutex_);
|
|
|
++ DCHECK(file_.IsValid()) << __func__ << " called on invalid file";
|
|
|
++
|
|
|
++ int written_bytes = file_.Write(file_offset, buffer->Data(), write_size);
|
|
|
++ base::File::Error error =
|
|
|
++ (written_bytes < 0) ? file_.GetLastFileError() : base::File::FILE_OK;
|
|
|
++ int64_t actual_file_length_on_failure = 0;
|
|
|
++ if (written_bytes < write_size || error != base::File::FILE_OK) {
|
|
|
++ actual_file_length_on_failure = file_.GetLength();
|
|
|
++ if (actual_file_length_on_failure < 0 && error != base::File::FILE_OK)
|
|
|
++ error = file_.GetLastFileError();
|
|
|
++ }
|
|
|
++
|
|
|
++ return {actual_file_length_on_failure, written_bytes, error};
|
|
|
++ }
|
|
|
++
|
|
|
++ base::File::Error Flush() {
|
|
|
++ DCHECK(!IsMainThread());
|
|
|
++
|
|
|
++ WTF::MutexLocker mutex_locker(mutex_);
|
|
|
++ DCHECK(file_.IsValid()) << __func__ << " called on invalid file";
|
|
|
++
|
|
|
++ bool success = file_.Flush();
|
|
|
++ return success ? base::File::FILE_OK : file_.GetLastFileError();
|
|
|
++ }
|
|
|
++
|
|
|
++ private:
|
|
|
+ // Lock coordinating cross-thread access to the state.
|
|
|
+- WTF::Mutex mutex;
|
|
|
++ WTF::Mutex mutex_;
|
|
|
++
|
|
|
+ // The file on disk backing this NativeIOFile.
|
|
|
+- //
|
|
|
+- // The mutex is there to protect us against using the file after it was
|
|
|
+- // closed, and against OS-specific behavior around concurrent file access. It
|
|
|
+- // should never cause the main (JS) thread to block. This is because the mutex
|
|
|
+- // is only taken on the main thread in CloseBackingFile(), which is called
|
|
|
+- // when the NativeIOFile is destroyed (which implies there's no pending I/O
|
|
|
+- // operation, because all I/O operations hold onto a Persistent<NativeIOFile>)
|
|
|
+- // and when the mojo pipe is closed, which currently only happens when the JS
|
|
|
+- // context is being torn down.
|
|
|
+- //
|
|
|
+- // TODO(rstz): Is it possible and worthwhile to remove the mutex and rely
|
|
|
+- // exclusively on |NativeIOFile::io_pending_|, or remove
|
|
|
+- // |NativeIOFile::io_pending_| in favor of the mutex (might be harder)?
|
|
|
+- base::File file GUARDED_BY(mutex);
|
|
|
++ base::File file_ GUARDED_BY(mutex_);
|
|
|
+ };
|
|
|
+
|
|
|
+ NativeIOFile::NativeIOFile(
|
|
|
+@@ -81,7 +218,7 @@ NativeIOFile::NativeIOFile(
|
|
|
+ NativeIOCapacityTracker* capacity_tracker,
|
|
|
+ ExecutionContext* execution_context)
|
|
|
+ : file_length_(backing_file_length),
|
|
|
+- file_state_(std::make_unique<FileState>(std::move(backing_file))),
|
|
|
++ file_state_(base::MakeRefCounted<FileState>(std::move(backing_file))),
|
|
|
+ // TODO(pwnall): Get a dedicated queue when the specification matures.
|
|
|
+ resolver_task_runner_(
|
|
|
+ execution_context->GetTaskRunner(TaskType::kMiscPlatformAPI)),
|
|
|
+@@ -94,7 +231,7 @@ NativeIOFile::NativeIOFile(
|
|
|
+ }
|
|
|
+
|
|
|
+ NativeIOFile::~NativeIOFile() {
|
|
|
+- // Needed to avoid having the base::File destructor close the file descriptor
|
|
|
++ // Needed to avoid having the FileState destructor close the file descriptor
|
|
|
+ // synchronously on the main thread.
|
|
|
+ CloseBackingFile();
|
|
|
+ }
|
|
|
+@@ -114,6 +251,9 @@ ScriptPromise NativeIOFile::close(ScriptState* script_state) {
|
|
|
+ queued_close_resolver_ = resolver;
|
|
|
+
|
|
|
+ if (!io_pending_) {
|
|
|
++ DCHECK(file_state_)
|
|
|
++ << "file_state_ nulled out without setting closed_ or io_pending_";
|
|
|
++
|
|
|
+ // Pretend that a close() promise was queued behind an I/O operation, and
|
|
|
+ // the operation just finished. This is less logic than handling the
|
|
|
+ // non-queued case separately.
|
|
|
+@@ -138,18 +278,15 @@ ScriptPromise NativeIOFile::getLength(ScriptState* script_state,
|
|
|
+ "The file was already closed"));
|
|
|
+ return ScriptPromise();
|
|
|
+ }
|
|
|
+- io_pending_ = true;
|
|
|
++ DCHECK(file_state_)
|
|
|
++ << "file_state_ nulled out without setting closed_ or io_pending_";
|
|
|
+
|
|
|
++ io_pending_ = true;
|
|
|
+ auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
|
|
|
+- // CrossThreadUnretained() is safe here because the NativeIOFile::FileState
|
|
|
+- // instance is owned by this NativeIOFile, which is also passed to the task
|
|
|
+- // via WrapCrossThreadPersistent. Therefore, the FileState instance is
|
|
|
+- // guaranteed to remain alive during the task's execution.
|
|
|
+ worker_pool::PostTask(
|
|
|
+ FROM_HERE, {base::MayBlock()},
|
|
|
+ CrossThreadBindOnce(&DoGetLength, WrapCrossThreadPersistent(this),
|
|
|
+- WrapCrossThreadPersistent(resolver),
|
|
|
+- CrossThreadUnretained(file_state_.get()),
|
|
|
++ WrapCrossThreadPersistent(resolver), file_state_,
|
|
|
+ resolver_task_runner_));
|
|
|
+ return resolver->Promise();
|
|
|
+ }
|
|
|
+@@ -175,6 +312,9 @@ ScriptPromise NativeIOFile::setLength(ScriptState* script_state,
|
|
|
+ "The file was already closed"));
|
|
|
+ return ScriptPromise();
|
|
|
+ }
|
|
|
++ DCHECK(file_state_)
|
|
|
++ << "file_state_ nulled out without setting closed_ or io_pending_";
|
|
|
++
|
|
|
+ int64_t expected_length = base::as_signed(new_length);
|
|
|
+
|
|
|
+ DCHECK_GE(expected_length, 0);
|
|
|
+@@ -201,8 +341,8 @@ ScriptPromise NativeIOFile::setLength(ScriptState* script_state,
|
|
|
+ }
|
|
|
+ file_length_ = expected_length;
|
|
|
+ }
|
|
|
+- io_pending_ = true;
|
|
|
+
|
|
|
++ io_pending_ = true;
|
|
|
+ auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
|
|
|
+
|
|
|
+ #if defined(OS_MAC)
|
|
|
+@@ -217,26 +357,19 @@ ScriptPromise NativeIOFile::setLength(ScriptState* script_state,
|
|
|
+ // To preserve this invariant, we pass this file's handle to the browser
|
|
|
+ // process during the SetLength() mojo call, and the browser passes it back
|
|
|
+ // when the call completes.
|
|
|
+- {
|
|
|
+- WTF::MutexLocker locker(file_state_->mutex);
|
|
|
+- backend_file_->SetLength(
|
|
|
+- expected_length, std::move(file_state_->file),
|
|
|
+- WTF::Bind(&NativeIOFile::DidSetLengthIpc, WrapPersistent(this),
|
|
|
+- WrapPersistent(resolver)));
|
|
|
+- }
|
|
|
++ base::File file = file_state_->TakeFile();
|
|
|
++ backend_file_->SetLength(
|
|
|
++ expected_length, std::move(file),
|
|
|
++ WTF::Bind(&NativeIOFile::DidSetLengthIpc, WrapPersistent(this),
|
|
|
++ WrapPersistent(resolver)));
|
|
|
+ return resolver->Promise();
|
|
|
+ }
|
|
|
+ #endif // defined(OS_MAC)
|
|
|
+
|
|
|
+- // CrossThreadUnretained() is safe here because the NativeIOFile::FileState
|
|
|
+- // instance is owned by this NativeIOFile, which is also passed to the task
|
|
|
+- // via WrapCrossThreadPersistent. Therefore, the FileState instance is
|
|
|
+- // guaranteed to remain alive during the task's execution.
|
|
|
+ worker_pool::PostTask(
|
|
|
+ FROM_HERE, {base::MayBlock()},
|
|
|
+ CrossThreadBindOnce(&DoSetLength, WrapCrossThreadPersistent(this),
|
|
|
+- WrapCrossThreadPersistent(resolver),
|
|
|
+- CrossThreadUnretained(file_state_.get()),
|
|
|
++ WrapCrossThreadPersistent(resolver), file_state_,
|
|
|
+ resolver_task_runner_, expected_length));
|
|
|
+ return resolver->Promise();
|
|
|
+ }
|
|
|
+@@ -258,6 +391,8 @@ ScriptPromise NativeIOFile::read(ScriptState* script_state,
|
|
|
+ "The file was already closed"));
|
|
|
+ return ScriptPromise();
|
|
|
+ }
|
|
|
++ DCHECK(file_state_)
|
|
|
++ << "file_state_ nulled out without setting closed_ or io_pending_";
|
|
|
+
|
|
|
+ // TODO(pwnall): This assignment should move right before the
|
|
|
+ // worker_pool::PostTask() call.
|
|
|
+@@ -281,16 +416,10 @@ ScriptPromise NativeIOFile::read(ScriptState* script_state,
|
|
|
+ DCHECK(buffer->IsDetached());
|
|
|
+
|
|
|
+ auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
|
|
|
+- // The first CrossThreadUnretained() is safe here because the
|
|
|
+- // NativeIOFile::FileState instance is owned by this NativeIOFile, which is
|
|
|
+- // also passed to the task via WrapCrossThreadPersistent. Therefore, the
|
|
|
+- // FileState instance is guaranteed to remain alive during the task's
|
|
|
+- // execution.
|
|
|
+ worker_pool::PostTask(
|
|
|
+ FROM_HERE, {base::MayBlock()},
|
|
|
+ CrossThreadBindOnce(&DoRead, WrapCrossThreadPersistent(this),
|
|
|
+- WrapCrossThreadPersistent(resolver),
|
|
|
+- CrossThreadUnretained(file_state_.get()),
|
|
|
++ WrapCrossThreadPersistent(resolver), file_state_,
|
|
|
+ resolver_task_runner_, std::move(result_buffer_data),
|
|
|
+ file_offset, read_size));
|
|
|
+ return resolver->Promise();
|
|
|
+@@ -313,6 +442,8 @@ ScriptPromise NativeIOFile::write(ScriptState* script_state,
|
|
|
+ "The file was already closed"));
|
|
|
+ return ScriptPromise();
|
|
|
+ }
|
|
|
++ DCHECK(file_state_)
|
|
|
++ << "file_state_ nulled out without setting closed_ or io_pending_";
|
|
|
+
|
|
|
+ int write_size = NativeIOOperationSize(*buffer);
|
|
|
+ int64_t write_end_offset;
|
|
|
+@@ -346,6 +477,14 @@ ScriptPromise NativeIOFile::write(ScriptState* script_state,
|
|
|
+ file_length_ = write_end_offset;
|
|
|
+ }
|
|
|
+
|
|
|
++ // TODO(pwnall): This assignment should move right before the
|
|
|
++ // worker_pool::PostTask() call.
|
|
|
++ //
|
|
|
++ // `io_pending_` should only be set to true when we know for sure we'll post a
|
|
|
++ // task that eventually results in getting `io_pending_` set back to false.
|
|
|
++ // Having `io_pending_` set to true in an early return case (rejecting with an
|
|
|
++ // exception) leaves the NativeIOFile "stuck" in a state where all future I/O
|
|
|
++ // method calls will reject.
|
|
|
+ io_pending_ = true;
|
|
|
+
|
|
|
+ std::unique_ptr<NativeIODataBuffer> result_buffer_data =
|
|
|
+@@ -358,16 +497,10 @@ ScriptPromise NativeIOFile::write(ScriptState* script_state,
|
|
|
+ DCHECK(buffer->IsDetached());
|
|
|
+
|
|
|
+ auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
|
|
|
+- // The first CrossThreadUnretained() is safe here because the
|
|
|
+- // NativeIOFile::FileState instance is owned by this NativeIOFile, which is
|
|
|
+- // also passed to the task via WrapCrossThreadPersistent. Therefore, the
|
|
|
+- // FileState instance is guaranteed to remain alive during the task's
|
|
|
+- // execution.
|
|
|
+ worker_pool::PostTask(
|
|
|
+ FROM_HERE, {base::MayBlock()},
|
|
|
+ CrossThreadBindOnce(&DoWrite, WrapCrossThreadPersistent(this),
|
|
|
+- WrapCrossThreadPersistent(resolver),
|
|
|
+- CrossThreadUnretained(file_state_.get()),
|
|
|
++ WrapCrossThreadPersistent(resolver), file_state_,
|
|
|
+ resolver_task_runner_, std::move(result_buffer_data),
|
|
|
+ file_offset, write_size));
|
|
|
+ return resolver->Promise();
|
|
|
+@@ -391,18 +524,15 @@ ScriptPromise NativeIOFile::flush(ScriptState* script_state,
|
|
|
+ "The file was already closed"));
|
|
|
+ return ScriptPromise();
|
|
|
+ }
|
|
|
+- io_pending_ = true;
|
|
|
++ DCHECK(file_state_)
|
|
|
++ << "file_state_ nulled out without setting closed_ or io_pending_";
|
|
|
+
|
|
|
++ io_pending_ = true;
|
|
|
+ auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
|
|
|
+- // CrossThreadUnretained() is safe here because the NativeIOFile::FileState
|
|
|
+- // instance is owned by this NativeIOFile, which is also passed to the task
|
|
|
+- // via WrapCrossThreadPersistent. Therefore, the FileState instance is
|
|
|
+- // guaranteed to remain alive during the task's execution.
|
|
|
+ worker_pool::PostTask(
|
|
|
+ FROM_HERE, {base::MayBlock()},
|
|
|
+ CrossThreadBindOnce(&DoFlush, WrapCrossThreadPersistent(this),
|
|
|
+- WrapCrossThreadPersistent(resolver),
|
|
|
+- CrossThreadUnretained(file_state_.get()),
|
|
|
++ WrapCrossThreadPersistent(resolver), file_state_,
|
|
|
+ resolver_task_runner_));
|
|
|
+ return resolver->Promise();
|
|
|
+ }
|
|
|
+@@ -430,28 +560,29 @@ void NativeIOFile::DispatchQueuedClose() {
|
|
|
+ ScriptPromiseResolver* resolver = queued_close_resolver_;
|
|
|
+ queued_close_resolver_ = nullptr;
|
|
|
+
|
|
|
++ scoped_refptr<FileState> file_state = std::move(file_state_);
|
|
|
++ DCHECK(!file_state_);
|
|
|
++
|
|
|
+ worker_pool::PostTask(
|
|
|
+ FROM_HERE, {base::MayBlock()},
|
|
|
+ CrossThreadBindOnce(&DoClose, WrapCrossThreadPersistent(this),
|
|
|
+ WrapCrossThreadPersistent(resolver),
|
|
|
+- CrossThreadUnretained(file_state_.get()),
|
|
|
+- resolver_task_runner_));
|
|
|
++ std::move(file_state), resolver_task_runner_));
|
|
|
+ }
|
|
|
+
|
|
|
+ // static
|
|
|
+ void NativeIOFile::DoClose(
|
|
|
+ CrossThreadPersistent<NativeIOFile> native_io_file,
|
|
|
+ CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+- NativeIOFile::FileState* file_state,
|
|
|
++ scoped_refptr<NativeIOFile::FileState> file_state,
|
|
|
+ scoped_refptr<base::SequencedTaskRunner> resolver_task_runner) {
|
|
|
+ DCHECK(!IsMainThread()) << "File I/O should not happen on the main thread";
|
|
|
++ DCHECK(file_state);
|
|
|
++ DCHECK(file_state->IsValid())
|
|
|
++ << "File I/O operation queued after file closed";
|
|
|
++ DCHECK(resolver_task_runner);
|
|
|
+
|
|
|
+- {
|
|
|
+- WTF::MutexLocker locker(file_state->mutex);
|
|
|
+- DCHECK(file_state->file.IsValid())
|
|
|
+- << "file I/O operation queued after file closed";
|
|
|
+- file_state->file.Close();
|
|
|
+- }
|
|
|
++ file_state->Close();
|
|
|
+
|
|
|
+ PostCrossThreadTask(
|
|
|
+ *resolver_task_runner, FROM_HERE,
|
|
|
+@@ -482,19 +613,17 @@ void NativeIOFile::DidClose(
|
|
|
+ void NativeIOFile::DoGetLength(
|
|
|
+ CrossThreadPersistent<NativeIOFile> native_io_file,
|
|
|
+ CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+- NativeIOFile::FileState* file_state,
|
|
|
++ scoped_refptr<NativeIOFile::FileState> file_state,
|
|
|
+ scoped_refptr<base::SequencedTaskRunner> resolver_task_runner) {
|
|
|
+ DCHECK(!IsMainThread()) << "File I/O should not happen on the main thread";
|
|
|
++ DCHECK(file_state);
|
|
|
++ DCHECK(file_state->IsValid())
|
|
|
++ << "File I/O operation queued after file closed";
|
|
|
++ DCHECK(resolver_task_runner);
|
|
|
++
|
|
|
++ int64_t length;
|
|
|
+ base::File::Error get_length_error;
|
|
|
+- int64_t length = -1;
|
|
|
+- {
|
|
|
+- WTF::MutexLocker mutex_locker(file_state->mutex);
|
|
|
+- DCHECK(file_state->file.IsValid())
|
|
|
+- << "file I/O operation queued after file closed";
|
|
|
+- length = file_state->file.GetLength();
|
|
|
+- get_length_error = (length < 0) ? file_state->file.GetLastFileError()
|
|
|
+- : base::File::FILE_OK;
|
|
|
+- }
|
|
|
++ std::tie(length, get_length_error) = file_state->GetLength();
|
|
|
+
|
|
|
+ PostCrossThreadTask(
|
|
|
+ *resolver_task_runner, FROM_HERE,
|
|
|
+@@ -541,22 +670,20 @@ void NativeIOFile::DidGetLength(
|
|
|
+ void NativeIOFile::DoSetLength(
|
|
|
+ CrossThreadPersistent<NativeIOFile> native_io_file,
|
|
|
+ CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+- NativeIOFile::FileState* file_state,
|
|
|
++ scoped_refptr<NativeIOFile::FileState> file_state,
|
|
|
+ scoped_refptr<base::SequencedTaskRunner> resolver_task_runner,
|
|
|
+ int64_t expected_length) {
|
|
|
+ DCHECK(!IsMainThread()) << "File I/O should not happen on the main thread";
|
|
|
++ DCHECK(file_state);
|
|
|
++ DCHECK(file_state->IsValid())
|
|
|
++ << "File I/O operation queued after file closed";
|
|
|
++ DCHECK(resolver_task_runner);
|
|
|
++ DCHECK_GE(expected_length, 0);
|
|
|
+
|
|
|
+- base::File::Error set_length_error;
|
|
|
+ int64_t actual_length;
|
|
|
+- {
|
|
|
+- WTF::MutexLocker mutex_locker(file_state->mutex);
|
|
|
+- DCHECK(file_state->file.IsValid())
|
|
|
+- << "file I/O operation queued after file closed";
|
|
|
+- bool success = file_state->file.SetLength(expected_length);
|
|
|
+- set_length_error =
|
|
|
+- success ? base::File::FILE_OK : file_state->file.GetLastFileError();
|
|
|
+- actual_length = success ? expected_length : file_state->file.GetLength();
|
|
|
+- }
|
|
|
++ base::File::Error set_length_error;
|
|
|
++ std::tie(actual_length, set_length_error) =
|
|
|
++ file_state->SetLength(expected_length);
|
|
|
+
|
|
|
+ PostCrossThreadTask(
|
|
|
+ *resolver_task_runner, FROM_HERE,
|
|
|
+@@ -619,10 +746,7 @@ void NativeIOFile::DidSetLengthIpc(
|
|
|
+ int64_t actual_length,
|
|
|
+ mojom::blink::NativeIOErrorPtr set_length_error) {
|
|
|
+ DCHECK(backing_file.IsValid()) << "browser returned closed file";
|
|
|
+- {
|
|
|
+- WTF::MutexLocker locker(file_state_->mutex);
|
|
|
+- file_state_->file = std::move(backing_file);
|
|
|
+- }
|
|
|
++ file_state_->SetFile(std::move(backing_file));
|
|
|
+ ScriptState* script_state = resolver->GetScriptState();
|
|
|
+
|
|
|
+ DCHECK(io_pending_) << "I/O operation performed without io_pending_ set";
|
|
|
+@@ -673,13 +797,15 @@ void NativeIOFile::DidSetLengthIpc(
|
|
|
+ void NativeIOFile::DoRead(
|
|
|
+ CrossThreadPersistent<NativeIOFile> native_io_file,
|
|
|
+ CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+- NativeIOFile::FileState* file_state,
|
|
|
++ scoped_refptr<NativeIOFile::FileState> file_state,
|
|
|
+ scoped_refptr<base::SequencedTaskRunner> resolver_task_runner,
|
|
|
+ std::unique_ptr<NativeIODataBuffer> result_buffer_data,
|
|
|
+ uint64_t file_offset,
|
|
|
+ int read_size) {
|
|
|
+ DCHECK(!IsMainThread()) << "File I/O should not happen on the main thread";
|
|
|
+-
|
|
|
++ DCHECK(file_state);
|
|
|
++ DCHECK(file_state->IsValid())
|
|
|
++ << "File I/O operation queued after file closed";
|
|
|
+ DCHECK(resolver_task_runner);
|
|
|
+ DCHECK(result_buffer_data);
|
|
|
+ DCHECK(result_buffer_data->IsValid());
|
|
|
+@@ -690,15 +816,8 @@ void NativeIOFile::DoRead(
|
|
|
+
|
|
|
+ int read_bytes;
|
|
|
+ base::File::Error read_error;
|
|
|
+- {
|
|
|
+- WTF::MutexLocker mutex_locker(file_state->mutex);
|
|
|
+- DCHECK(file_state->file.IsValid())
|
|
|
+- << "file I/O operation queued after file closed";
|
|
|
+- read_bytes = file_state->file.Read(file_offset, result_buffer_data->Data(),
|
|
|
+- read_size);
|
|
|
+- read_error = (read_bytes < 0) ? file_state->file.GetLastFileError()
|
|
|
+- : base::File::FILE_OK;
|
|
|
+- }
|
|
|
++ std::tie(read_bytes, read_error) =
|
|
|
++ file_state->Read(result_buffer_data.get(), file_offset, read_size);
|
|
|
+
|
|
|
+ PostCrossThreadTask(
|
|
|
+ *resolver_task_runner, FROM_HERE,
|
|
|
+@@ -743,12 +862,15 @@ void NativeIOFile::DidRead(
|
|
|
+ void NativeIOFile::DoWrite(
|
|
|
+ CrossThreadPersistent<NativeIOFile> native_io_file,
|
|
|
+ CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+- NativeIOFile::FileState* file_state,
|
|
|
++ scoped_refptr<NativeIOFile::FileState> file_state,
|
|
|
+ scoped_refptr<base::SequencedTaskRunner> resolver_task_runner,
|
|
|
+ std::unique_ptr<NativeIODataBuffer> result_buffer_data,
|
|
|
+ uint64_t file_offset,
|
|
|
+ int write_size) {
|
|
|
+ DCHECK(!IsMainThread()) << "File I/O should not happen on the main thread";
|
|
|
++ DCHECK(file_state);
|
|
|
++ DCHECK(file_state->IsValid())
|
|
|
++ << "File I/O operation queued after file closed";
|
|
|
+ DCHECK(resolver_task_runner);
|
|
|
+ DCHECK(result_buffer_data);
|
|
|
+ DCHECK(result_buffer_data->IsValid());
|
|
|
+@@ -760,22 +882,8 @@ void NativeIOFile::DoWrite(
|
|
|
+ int written_bytes;
|
|
|
+ int64_t actual_file_length_on_failure = 0;
|
|
|
+ base::File::Error write_error;
|
|
|
+- {
|
|
|
+- WTF::MutexLocker mutex_locker(file_state->mutex);
|
|
|
+- DCHECK(file_state->file.IsValid())
|
|
|
+- << "file I/O operation queued after file closed";
|
|
|
+- written_bytes = file_state->file.Write(
|
|
|
+- file_offset, result_buffer_data->Data(), write_size);
|
|
|
+- write_error = (written_bytes < 0) ? file_state->file.GetLastFileError()
|
|
|
+- : base::File::FILE_OK;
|
|
|
+- if (written_bytes < write_size || write_error != base::File::FILE_OK) {
|
|
|
+- actual_file_length_on_failure = file_state->file.GetLength();
|
|
|
+- if (actual_file_length_on_failure < 0 &&
|
|
|
+- write_error != base::File::FILE_OK) {
|
|
|
+- write_error = file_state->file.GetLastFileError();
|
|
|
+- }
|
|
|
+- }
|
|
|
+- }
|
|
|
++ std::tie(actual_file_length_on_failure, written_bytes, write_error) =
|
|
|
++ file_state->Write(result_buffer_data.get(), file_offset, write_size);
|
|
|
+
|
|
|
+ PostCrossThreadTask(
|
|
|
+ *resolver_task_runner, FROM_HERE,
|
|
|
+@@ -846,18 +954,14 @@ void NativeIOFile::DidWrite(
|
|
|
+ void NativeIOFile::DoFlush(
|
|
|
+ CrossThreadPersistent<NativeIOFile> native_io_file,
|
|
|
+ CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+- NativeIOFile::FileState* file_state,
|
|
|
++ scoped_refptr<NativeIOFile::FileState> file_state,
|
|
|
+ scoped_refptr<base::SequencedTaskRunner> resolver_task_runner) {
|
|
|
+ DCHECK(!IsMainThread()) << "File I/O should not happen on the main thread";
|
|
|
+- base::File::Error flush_error;
|
|
|
+- {
|
|
|
+- WTF::MutexLocker mutex_locker(file_state->mutex);
|
|
|
+- DCHECK(file_state->file.IsValid())
|
|
|
+- << "file I/O operation queued after file closed";
|
|
|
+- bool success = file_state->file.Flush();
|
|
|
+- flush_error =
|
|
|
+- success ? base::File::FILE_OK : file_state->file.GetLastFileError();
|
|
|
+- }
|
|
|
++ DCHECK(file_state);
|
|
|
++ DCHECK(file_state->IsValid())
|
|
|
++ << "File I/O operation queued after file closed";
|
|
|
++
|
|
|
++ base::File::Error flush_error = file_state->Flush();
|
|
|
+
|
|
|
+ PostCrossThreadTask(
|
|
|
+ *resolver_task_runner, FROM_HERE,
|
|
|
+@@ -887,20 +991,23 @@ void NativeIOFile::DidFlush(
|
|
|
+
|
|
|
+ void NativeIOFile::CloseBackingFile() {
|
|
|
+ closed_ = true;
|
|
|
+- file_state_->mutex.lock();
|
|
|
+- base::File backing_file = std::move(file_state_->file);
|
|
|
+- file_state_->mutex.unlock();
|
|
|
+
|
|
|
+- if (!backing_file.IsValid()) {
|
|
|
++ if (!file_state_) {
|
|
|
+ // Avoid posting a cross-thread task if the file is already closed. This is
|
|
|
+ // the expected path.
|
|
|
+ return;
|
|
|
+ }
|
|
|
+
|
|
|
+- worker_pool::PostTask(
|
|
|
+- FROM_HERE, {base::MayBlock()},
|
|
|
+- CrossThreadBindOnce([](base::File file) { file.Close(); },
|
|
|
+- std::move(backing_file)));
|
|
|
++ scoped_refptr<FileState> file_state = std::move(file_state_);
|
|
|
++ DCHECK(!file_state_);
|
|
|
++
|
|
|
++ worker_pool::PostTask(FROM_HERE, {base::MayBlock()},
|
|
|
++ CrossThreadBindOnce(
|
|
|
++ [](scoped_refptr<FileState> file_state) {
|
|
|
++ DCHECK(file_state);
|
|
|
++ file_state->Close();
|
|
|
++ },
|
|
|
++ std::move(file_state)));
|
|
|
+ }
|
|
|
+
|
|
|
+ } // namespace blink
|
|
|
+diff --git a/third_party/blink/renderer/modules/native_io/native_io_file.h b/third_party/blink/renderer/modules/native_io/native_io_file.h
|
|
|
+index 8ae49ebc2d36d547d152d4e56192e30f8cacd641..95d2da4ac3e1859a0abc21ea15d269758eed1681 100644
|
|
|
+--- a/third_party/blink/renderer/modules/native_io/native_io_file.h
|
|
|
++++ b/third_party/blink/renderer/modules/native_io/native_io_file.h
|
|
|
+@@ -67,12 +67,7 @@ class NativeIOFile final : public ScriptWrappable {
|
|
|
+ void Trace(Visitor* visitor) const override;
|
|
|
+
|
|
|
+ private:
|
|
|
+- // Data accessed on the threads that do file I/O.
|
|
|
+- //
|
|
|
+- // Instances are allocated on the PartitionAlloc heap. Instances are initially
|
|
|
+- // constructed on Blink's main thread, or on a worker thread. Afterwards,
|
|
|
+- // instances are only accessed on dedicated threads that do blocking file I/O.
|
|
|
+- struct FileState;
|
|
|
++ class FileState;
|
|
|
+
|
|
|
+ // Called when the mojo backend disconnects.
|
|
|
+ void OnBackendDisconnect();
|
|
|
+@@ -84,7 +79,7 @@ class NativeIOFile final : public ScriptWrappable {
|
|
|
+ static void DoClose(
|
|
|
+ CrossThreadPersistent<NativeIOFile> native_io_file,
|
|
|
+ CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+- NativeIOFile::FileState* file_state,
|
|
|
++ scoped_refptr<NativeIOFile::FileState> file_state,
|
|
|
+ scoped_refptr<base::SequencedTaskRunner> file_task_runner);
|
|
|
+ // Performs the post file I/O part of close(), on the main thread.
|
|
|
+ void DidClose(CrossThreadPersistent<ScriptPromiseResolver> resolver);
|
|
|
+@@ -93,7 +88,7 @@ class NativeIOFile final : public ScriptWrappable {
|
|
|
+ static void DoGetLength(
|
|
|
+ CrossThreadPersistent<NativeIOFile> native_io_file,
|
|
|
+ CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+- NativeIOFile::FileState* file_state,
|
|
|
++ scoped_refptr<NativeIOFile::FileState> file_state,
|
|
|
+ scoped_refptr<base::SequencedTaskRunner> file_task_runner);
|
|
|
+ // Performs the post file I/O part of getLength(), on the main thread.
|
|
|
+ void DidGetLength(CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+@@ -104,7 +99,7 @@ class NativeIOFile final : public ScriptWrappable {
|
|
|
+ static void DoSetLength(
|
|
|
+ CrossThreadPersistent<NativeIOFile> native_io_file,
|
|
|
+ CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+- NativeIOFile::FileState* file_state,
|
|
|
++ scoped_refptr<NativeIOFile::FileState> file_state,
|
|
|
+ scoped_refptr<base::SequencedTaskRunner> file_task_runner,
|
|
|
+ int64_t expected_length);
|
|
|
+ // Performs the post file I/O part of setLength(), on the main thread.
|
|
|
+@@ -128,7 +123,7 @@ class NativeIOFile final : public ScriptWrappable {
|
|
|
+ // Performs the file I/O part of read(), off the main thread.
|
|
|
+ static void DoRead(CrossThreadPersistent<NativeIOFile> native_io_file,
|
|
|
+ CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+- NativeIOFile::FileState* file_state,
|
|
|
++ scoped_refptr<NativeIOFile::FileState> file_state,
|
|
|
+ scoped_refptr<base::SequencedTaskRunner> file_task_runner,
|
|
|
+ std::unique_ptr<NativeIODataBuffer> result_buffer_data,
|
|
|
+ uint64_t file_offset,
|
|
|
+@@ -143,7 +138,7 @@ class NativeIOFile final : public ScriptWrappable {
|
|
|
+ static void DoWrite(
|
|
|
+ CrossThreadPersistent<NativeIOFile> native_io_file,
|
|
|
+ CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+- NativeIOFile::FileState* file_state,
|
|
|
++ scoped_refptr<NativeIOFile::FileState> file_state,
|
|
|
+ scoped_refptr<base::SequencedTaskRunner> resolver_task_runner,
|
|
|
+ std::unique_ptr<NativeIODataBuffer> result_buffer_data,
|
|
|
+ uint64_t file_offset,
|
|
|
+@@ -163,7 +158,7 @@ class NativeIOFile final : public ScriptWrappable {
|
|
|
+ static void DoFlush(
|
|
|
+ CrossThreadPersistent<NativeIOFile> native_io_file,
|
|
|
+ CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+- NativeIOFile::FileState* file_state,
|
|
|
++ scoped_refptr<NativeIOFile::FileState> file_state,
|
|
|
+ scoped_refptr<base::SequencedTaskRunner> file_task_runner);
|
|
|
+ // Performs the post file-I/O part of flush(), on the main thread.
|
|
|
+ void DidFlush(CrossThreadPersistent<ScriptPromiseResolver> resolver,
|
|
|
+@@ -210,8 +205,13 @@ class NativeIOFile final : public ScriptWrappable {
|
|
|
+ // TODO(rstz): Consider moving this variable into `file_state_`
|
|
|
+ int64_t file_length_ = 0;
|
|
|
+
|
|
|
+- // See NativeIOFile::FileState, declared above.
|
|
|
+- const std::unique_ptr<FileState> file_state_;
|
|
|
++ // Points to a NativeIOFile::FileState while the underlying file is open.
|
|
|
++ //
|
|
|
++ // When the underlying file is closed, this pointer is nulled out, and the
|
|
|
++ // FileState instance is passed to a different thread, where the closing
|
|
|
++ // happens. This avoids having any I/O performed by the base::File::Close()
|
|
|
++ // jank the JavaScript thread that owns this NativeIOFile instance.
|
|
|
++ scoped_refptr<FileState> file_state_;
|
|
|
+
|
|
|
+ // Schedules resolving Promises with file I/O results.
|
|
|
+ const scoped_refptr<base::SequencedTaskRunner> resolver_task_runner_;
|