Merge pull request #11141 from murgatroid99/node_write_after_end_race_fix

Fix race between destroying call after status and handling write failure

2 files changed, 18 insertions, 2 deletions

diff --git a/src/node/ext/call.cc b/src/node/ext/call.cc
index 8877995f8f..2cc9f63b65 100644
--- a/src/node/ext/call.cc
+++ b/src/node/ext/call.cc
@@ -721,6 +721,14 @@ NAN_METHOD(Call::StartBatch) {
   }
   Local<Function> callback_func = info[1].As<Function>();
   Call *call = ObjectWrap::Unwrap<Call>(info.This());
+  if (call->wrapped_call == NULL) {
+    /* This implies that the call has completed and has been destroyed. To emulate
+     * previous behavior, we should call the callback immediately with an error,
+     * as though the batch had failed in core */
+    Local<Value> argv[] = {Nan::Error("The async function failed because the call has completed")};
+    Nan::Call(callback_func, Nan::New<Object>(), 1, argv);
+    return;
+  }
   Local<Object> obj = Nan::To<Object>(info[0]).ToLocalChecked();
   Local<Array> keys = Nan::GetOwnPropertyNames(obj).ToLocalChecked();
   size_t nops = keys->Length();
diff --git a/src/node/src/client.js b/src/node/src/client.js
index 1aaf35c16c..2073d3eea8 100644
--- a/src/node/src/client.js
+++ b/src/node/src/client.js
@@ -100,6 +100,12 @@ function _write(chunk, encoding, callback) {
   /* jshint validthis: true */
   var batch = {};
   var message;
+  var self = this;
+  if (this.writeFailed) {
+    /* Once a write fails, just call the callback immediately to let the caller
+       flush any pending writes. */
+    setImmediate(callback);
+  }
   try {
     message = this.serialize(chunk);
   } catch (e) {
@@ -119,8 +125,10 @@ function _write(chunk, encoding, callback) {
   batch[grpc.opType.SEND_MESSAGE] = message;
   this.call.startBatch(batch, function(err, event) {
     if (err) {
-      // Something has gone wrong. Stop writing by failing to call callback
-      return;
+      /* Assume that the call is complete and that writing failed because a
+         status was received. In that case, set a flag to discard all future
+         writes */
+      self.writeFailed = true;
     }
     callback();
   });