search

davedoesdev / bpmux

Node stream multiplexing with back-pressure on each stream
MIT License
18 stars 2 forks source link

Duplexes not removed when TYPE_END is received #22

Open alexivanov opened 4 months ago

alexivanov commented 4 months ago

If the remote part of a connection closes a duplex by sending a TYPE_END packet via .end() the local duplex isn't removed / closed fully.

I believe the logic for handling TYPE_END packets should also destroy the duplex here:

https://github.com/davedoesdev/bpmux/blob/72ad2b1e057a898693a7b2074a84794e931363b5/index.js#L1017

Currently from the docs it doesn't appear to be a another way for detecting when the remote closes a connection apart from looking at the internal BPDuplex._ended.

davedoesdev commented 4 months ago

The local duplex will still remain open for writes and should be destroyed once its applicatoin calls .end()

alexivanov commented 4 months ago

@davedoesdev thanks! I guess then a question is how does one detect that the remote end has closed the duplex? For our use case we don’t want to keep half closed duplexes around.

davedoesdev commented 4 months ago

Ah I think I see what you mean now. You're not actively reading from the duplex so won't get an end event?

alexivanov commented 4 months ago

Yep, essentially that’s what happens in some cases. I haven't experimented with the native Duplex implementation but my guess is that an end (or related) event will be emitted if the other end closes the connection even when not actively reading?

davedoesdev commented 4 months ago

Native Duplex will only emit end when read() has returned null.

I do see an option for bpmux to diverge from this behaviour being useful, will be a separate feature though.

davedoesdev commented 4 months ago

Btw I'm updating bpmux to fix some issues with updated dependencies, I've left it to rot a bit.

alexivanov commented 4 months ago

Ah yes, it sounds like no end event will be emitted for a Duplex unless the data is completely consumed (Duplex end docs). This differs to the (default) TCP net.Socket implementation which emits an event then the other end closes the connection (net.Socket end docs)

You're right, this will be more of a feature to support this.