Uncaught Error [ERR_STREAM_DESTROYED]: Cannot call write after a stream was destroyed

[ibc]

Eventually this happened and the Node process exited with error (due to uncaught error event):

Error [ERR_STREAM_DESTROYED]: Cannot call write after a stream was destroyed
    at doWrite (_stream_writable.js:406:19)
    at clearBuffer (_stream_writable.js:540:7)
    at onwrite (_stream_writable.js:465:7)
    at association.SEND (/Users/me/myproject//node_modules/sctp/lib/sockets.js:141:7)
    at SEND.send.error (/Users/me/myproject//node_modules/sctp/lib/association.js:1659:11)
    at callbacks.forEach.cb (/Users/me/myproject//node_modules/sctp/lib/association.js:1336:13)
    at Array.forEach (<anonymous>)
    at SendWrap.endpoint._sendPacket [as callback] (/Users/me/myproject//node_modules/sctp/lib/association.js:1333:19)
    at SendWrap.afterSend [as oncomplete] (dgram.js:472:8)
  myproject:ERROR run() | terminating process [exit code:1, signal:undefined] +0ms
[ERROR] 11:18:26 Error [ERR_STREAM_DESTROYED]: Cannot call write after a stream was destroyed

[ibc]

I've been checking the code and (not 100% sure) I think that the SEND() method in association.js does not check whether the association has been closed after this.send() completes, so it invokes the callback and such may trigger a new sending of data when the stream has been destroyed.

Perhaps the Association should have a closed flag (or similar) so any async task or callback is discarded if this.closed === true? Well, IMHO the same may happen in other files, unless they can, instead, check the native socket or stream status before trying to use them.

[ibc]

I think that the problem is in the sctp Socket class (that extends Node Duplex). It's _write() method is provided with a callback (the onwrite callback). Eventually onwrite fires and, after that, the app closes the sctp Socket, but the callback is then executed and AFAIS it tries to write again, and hence the "Cannot call write after a stream was destroyed".

Does this ring any bell, @latysheff?

[latysheff]

Hi, @ibc, please try changing both _write() occurrences in sockets.js to pass not callback as is, but function:

() => {
      if (!this.destroyed) callback()
    }

Currently I have no environment to check if this works. In case of success will quickly publish an update.

[ibc]

There is no this.destroyed in sockets.js. There is a TODO that covers it but it's not set anywhere.

[ibc]

Could such a this.destroyed = true be set in Socket class within the _final() method?

  _final (callback) {
    /*
    This optional function will be called before the stream closes,
    delaying the 'finish' event until callback is called.
    This is useful to close resources or write buffered data
    before a stream ends.
    */
    // called by end()
    // TODO
    this.debugger.info('_final')
    this.destroyed = true; // <-----------------------
    if (this.association) {
      this.association.SHUTDOWN(callback)
    }
  }

[latysheff]

"destroyed" is a property of Duplex and Writable class, which are extended.

[ibc]

Oh, got it. Thanks, will send PR in a few minutes.

[latysheff]

Thank you, but please test it before :)

[ibc]

I've create a PR https://github.com/latysheff/node-sctp/pull/18 just for you to check (not to merge yet). I will also use that branch in my project and confirm it the crash is solved. Can you just please confirm whether the changes are ok according to what you suggested above?

BTE I've just seen the crash happen once, so it's not super easy to reproduce, but let's see.

[latysheff]

Yes, this is what is meant. Checking callback is truthy seems redundant, I believe it is always provided internally, but is ok.

[ibc]

No crashes. But somehow I've messed up the PR so I've created a new one:

I've create a PR https://github.com/latysheff/node-sctp/pull/19

May you merge and release, please?

[latysheff]

Done.

[ibc]

Thanks.