fix: infinite loop when connection is aborted before being accepted #1164
Conversation
diegomrsantos
changed the title
| self.acceptFuts = self.servers.mapIt(it.accept()) | ||
|
|
||
| let | ||
| finished = | ||
| try: | ||
| # Waits for any one of these futures to complete, indicating that a new connection has been accepted on one of the servers. | ||
| await one(self.acceptFuts) | ||
| except ValueError: | ||
| raise (ref TcpTransportError)(msg: "No listeners configured") |
There was a problem hiding this comment.
I think you should not rely on chronos one() function to understand that length of self.acceptFuts == 0, you should establish your own check and in case of ValueError returned from one() call you could raise assertion.
| self.acceptFuts = self.servers.mapIt(it.accept()) | ||
|
|
||
| let | ||
| finished = | ||
| try: | ||
| # Waits for any one of these futures to complete, indicating that a new connection has been accepted on one of the servers. |
There was a problem hiding this comment.
This await call missing Cancellation handler. Because you are using combinator one() which does not perform any cancellation on passed Futures, so in case of cancellation its possible that all it.accept() futures will be left unattended.
| except common.TransportError as exc: # Needed for chronos 4.0.0 support | ||
| raise (ref TcpTransportError)(msg: exc.msg, parent: exc) | ||
| except CancelledError as exc: | ||
| raise exc |
There was a problem hiding this comment.
In this handler you do not catch CatchableError anymore, so there is no need to save this default Cancellation handler. Of course in this specific case you should do something with acceptFuts array which could be left unattended.
Address the comments in #1164
self.acceptFuts[index] = self.servers[index].accept()wasn't being called when there was an error.