ttrpc icon indicating copy to clipboard operation
ttrpc copied to clipboard

stream: fix the map of streams leak

Open wllenyj opened this issue 2 years ago • 3 comments

In a connection, streams are added only, but not deleted. When replying to the last data, delete the stream from map.

And delete operations require concurrency.

Signed-off-by: wllenyj [email protected]

wllenyj avatar Jun 06 '22 18:06 wllenyj

Ahh yes, thanks

My original thought was to use a buffered channel to mark the removals to be handled in the receive go routine. The motivation being to avoid a locking data structure on the receive path. However, a flood of stream closing which fill up the buffer would likely make that solution unworkable. I'll try to consider other lock free ways we could handle this, but we should get this change in.

dmcgowan avatar Jun 08 '22 19:06 dmcgowan

@dmcgowan thanks. Streaming is a great project, the logic in it is so complex that it took me a lot of time to understand it, I admire your ability to come up with this stuff :)

wllenyj avatar Jun 09 '22 03:06 wllenyj

How did you notice the issue by the way? It would be great if we can write some autometed way to check that this part won't have leaks in future.

I added the test log locally. If we want to cover it by unit test, we need to save streams to serverConn in order to check it at some point later. But it is expected that there will be more modifications.

wllenyj avatar Jul 13 '22 08:07 wllenyj