race condition on stream close

[virtuald]

This is for a server-side stream. The stack trace I'm getting on the client is something like so:

Uncaught TypeError: Cannot read property 'data' of null

• onMessage
• Array.<anonymous>
• runCallbacks

This occurs when I'm clicking on my page rapidly, causing the stream to be opened/closed over and over again. I haven't tried to reproduce it using the example project.

It seems to me that perhaps the setTimeout(0) in detach.ts is the cause of the race condition, as there doesn't seem to be a mechanism to remove callbacks that are queued via the timeout.

One workaround would be for ts-protoc-gen to add a check to see if listeners is null in the generated onMessage callbacks. I suspect a similar race condition exists in onEnd also, which could be fixable via the same workaround.

[johanbrandhorst]

Hi @virtuald, thanks for the bug report. What we'd ideally need is a simple case that reproduces the issue. Could you try to reproduce it in the example or perhaps create a test that repeatedly calls the methods and reproduces the issue?

[virtuald]

Well, the easiest way to reproduce this is by calling cancel from inside the data callback. It seems to me that it should be safe to call cancel at any time.

https://github.com/virtuald/grpc-web/tree/cancel-race

book_service_pb_service.js:81 Uncaught TypeError: Cannot read property 'end' of null
    at onEnd (book_service_pb_service.js:81)
    at Array.<anonymous> (client.js:161)
    at runCallbacks (detach.js:10)
    at detach.js:16

Perhaps calling from inside the handler isn't something that should be done (I disagree -- it should be supported), so I created another example that does it from outside the handler too.

function queryBooks() {
  const client = new BookServiceClient(host);
  const queryBooksRequest = new QueryBooksRequest();
  queryBooksRequest.setAuthorPrefix("Geor");

  const s = client.queryBooks(queryBooksRequest);
  s.on("data", (message: Book) => {
    console.log('data received', message.toObject());
  });
  setTimeout( () => {
    console.log("timeout set");
    s.cancel();
    queryBooks();
  }, 0);
}

https://github.com/virtuald/grpc-web/tree/cancel-race2

timeout set
index.ts:39 timeout set
client:77 [WDS] Hot Module Replacement enabled.
index.ts:39 timeout set
index.ts:39 timeout set
index.ts:39 timeout set
book_service_pb_service.js:76 Uncaught TypeError: Cannot read property 'data' of null
    at onMessage (book_service_pb_service.js:76)
    at Array.<anonymous> (client.js:192)
    at runCallbacks (detach.js:10)
    at detach.js:34

Edit: Ha, I just realized that I provoked the race for onEnd and onMessage without realizing it.

[johanbrandhorst]

Thanks, that's great! Would you be interested in contributing a fix for this as well? I'm not sure we have any time to fix this in the short term.

[virtuald]

Done. I'd love if ya'll could take a look at that and my 2 PR's on ts-protoc-gen.

[johanbrandhorst]

Unfortunately I personally do not have maintainer powers over ts-protoc-gen, but I'll forward your request.