fix: race condition in NodeStreamLoader (#19869)

* fix: race condition in NodeStreamLoader

* nit: add comments

shell/browser/net/node_stream_loader.cc

@@ -71,12 +71,18 @@ void NodeStreamLoader::Start(network::ResourceResponseHead head) {
      base::BindRepeating(&NodeStreamLoader::NotifyComplete, weak, net::OK));
   On("error", base::BindRepeating(&NodeStreamLoader::NotifyComplete, weak,
                                   net::ERR_FAILED));
-  On("readable", base::BindRepeating(&NodeStreamLoader::ReadMore, weak));
+  On("readable", base::BindRepeating(&NodeStreamLoader::NotifyReadable, weak));
+}
+
+void NodeStreamLoader::NotifyReadable() {
+  if (!readable_)
+    ReadMore();
+  readable_ = true;
 }
 
 void NodeStreamLoader::NotifyComplete(int result) {
   // Wait until write finishes or fails.
-  if (is_writing_) {
+  if (is_reading_ || is_writing_) {
     ended_ = true;
     result_ = result;
     return;
@@ -87,19 +93,24 @@ void NodeStreamLoader::NotifyComplete(int result) {
 }
 
 void NodeStreamLoader::ReadMore() {
+  is_reading_ = true;
   // buffer = emitter.read()
   v8::MaybeLocal<v8::Value> ret = node::MakeCallback(
       isolate_, emitter_.Get(isolate_), "read", 0, nullptr, {0, 0});
 
   // If there is no buffer read, wait until |readable| is emitted again.
   v8::Local<v8::Value> buffer;
-  if (!ret.ToLocal(&buffer) || !node::Buffer::HasInstance(buffer))
+  if (!ret.ToLocal(&buffer) || !node::Buffer::HasInstance(buffer)) {
+    readable_ = false;
+    is_reading_ = false;
     return;
+  }
 
   // Hold the buffer until the write is done.
   buffer_.Reset(isolate_, buffer);
 
   // Write buffer to mojo pipe asyncronously.
+  is_reading_ = false;
   is_writing_ = true;
   producer_->Write(
       std::make_unique<mojo::StringDataSource>(
@@ -118,7 +129,7 @@ void NodeStreamLoader::DidWrite(MojoResult result) {
     return;
   }
 
-  if (result == MOJO_RESULT_OK)
+  if (result == MOJO_RESULT_OK && readable_)
     ReadMore();
   else
     NotifyComplete(net::ERR_FAILED);

shell/browser/net/node_stream_loader.h

@@ -39,6 +39,7 @@ class NodeStreamLoader : public network::mojom::URLLoader {
   using EventCallback = base::RepeatingCallback<void()>;
 
   void Start(network::ResourceResponseHead head);
+  void NotifyReadable();
   void NotifyComplete(int result);
   void ReadMore();
   void DidWrite(MojoResult result);
@@ -69,11 +70,19 @@ class NodeStreamLoader : public network::mojom::URLLoader {
   // Whether we are in the middle of write.
   bool is_writing_ = false;
 
+  // Whether we are in the middle of a stream.read().
+  bool is_reading_ = false;
+
   // When NotifyComplete is called while writing, we will save the result and
   // quit with it after the write is done.
   bool ended_ = false;
   int result_ = net::OK;
 
+  // When the stream emits the readable event, we only want to start reading
+  // data if the stream was not readable before, so we store the state in a
+  // flag.
+  bool readable_ = false;
+
   // Store the V8 callbacks to unsubscribe them later.
   std::map<std::string, v8::Global<v8::Value>> handlers_;