dominictarr / mux-demux

mutiplex-demultiplex multiple streams through a single text Stream
MIT License
179 stars 15 forks source link

mux-demux tries to write to stream after it's closed #7

Closed Raynos closed 12 years ago

Raynos commented 12 years ago

After a stream is closed I get a write message incoming and this throws a stream is not writable Error.

This could be a race condition with buffering data whilst opening and closing connections or a bug in my code.

As always running seaport-proxy service demo and repeatedly opening and closing connections get's this error to bubble up.

mux-demux would be a lot more friendly if it just drops connections that are in a corrupted state rather then throwing an exception and killing my server. Calling stream.end() instead of throw new Error("fix your race conditions") would be nicer.

function createStream(id, meta, opts) {
    console.log("[CREATE] creatingStream", id)
    var s = es.through(function (data) {
      if(!this.writable) {
        console.log("[NOT WRITABLE]", id, meta, opts)
        throw new Error('stream is not writable')
      }
      md.emit('data', [s.id, 'data', data])
    }, function () {
      md.emit('data', [s.id, 'end'])
      if (this.readable && !opts.allowHalfOpen && !this.ended) {
        this.emit("end")
      }
    })
dominictarr commented 12 years ago

If it's not writable that means that it's already emitted end, close or error. (yeah, there are too many ways a stream can end, but that will change in 0.9)

can you post a stacktrace?

if you are using pipe to write to the stream, it will check whether it is writable, and drop the write if it isn't.

does this crash on the server or on the browser?

Raynos commented 12 years ago

@dominictarr it crashes on the server.

I think I need a flag to change all the thrown errors for logging and closing streams as it's just not worth crashing my server for

Raynos commented 12 years ago

@dominictarr stack trace is https://gist.github.com/5b99bdb16cfd5e1936d5

dominictarr commented 12 years ago

@raynos hey we fixed this right? we probably just talked about it in IRC and forgot about closing the issue.

Raynos commented 12 years ago

im sure this is fixed too.

dominictarr commented 12 years ago

cool, closing.