search

Pylons / waitress

Waitress - A WSGI server for Python 3
https://docs.pylonsproject.org/projects/waitress/en/latest/
Other
1.44k stars 164 forks source link

Remove race condition when creating new HTTPChannel #435

Closed digitalresistor closed 3 months ago

digitalresistor commented 6 months ago

When we accept() a connection we get the remote address and we pass it to HTTPChannel, then in wasyncore.dispatcher we were calling getpeername() again to get the same remote address that we already had.

This would cause an issue if the remote client had sent data, but closed the connection before getpeername() because the wasyncore.dispatcher would set the channel to no longer being connected.

Further down the line this would cause issues whereby handle_write would loop over and over because it would mark itself as selectable but the socket would never actually get torn down.

Closes #418 Supersedes #419

djay commented 4 months ago

@digitalresistor it's been running for a few weeks with no problem. the pentests still happen and we haven't see any problems.

vondele commented 2 months ago

great, find and thanks for fixing this. We have been running a server at https://tests.stockfishchess.org/tests that almost reads like the description of the testcase in https://github.com/Pylons/waitress/issues/418#issuecomment-1716970514 and have seen this happen multiple times in the past, if the load on the server was high (with dropped connections by nginx / nginx reboots), almost on a daily basis (most recently discussed as https://github.com/official-stockfish/fishtest/issues/2094 ... but we had no real clue).

We'd be happy to upgrade waitress to an eventual 3.0.1 as mentioned in https://github.com/Pylons/waitress/commit/34956747f6d34755471b29157c3e99f707829371 . Is there a timeline for that (also in light of the green CI)?