Changes the silent connection timeouts to consider full-duplex communication flow #3616
Conversation
thsfs
requested review from
dsiganos,
clemahieu,
argakiig,
cryptocode and
theohax
Yes, that is right. |
|
@thsfs is this ready to merge? |
|
I suspect this is dead. Thiago is working on another solution that will focus on silence on tcp server side only. |
I'm intending to append a complement to this. It will check for is_server () instead of type == realtime, plus the related code for ensuring the socket runs on the server-side. |
thsfs
force-pushed
the
outgoing_data_fix
branch
from
b7066d2 to
a9b2bba
Compare
thsfs
dismissed stale reviews from clemahieu and theohax
Dismissed after new changes.
| auto is_silent = [this_l] (auto const & time) { | ||
| auto const silent_for_receiving = (time - this_l->last_receive_time_or_init) > this_l->silent_connection_tolerance_time.count (); | ||
| auto const silent_for_sending = (time - this_l->last_send_time_or_init) > this_l->silent_connection_tolerance_time.count (); | ||
| return silent_for_receiving && silent_for_sending; |
There was a problem hiding this comment.
This silent_for_receiving && silent_for_sending shouldn't be an "AND" relationship.
It should at least be an OR relationship but I am not sure if we have a guaranteed minimum reply gap.
|
Replaced by: #3623 |
The first implementation wrongly considered the communication to be half-duplex, so talking-only connections were getting shutdown as the checking time was only being set for the
async_read()function. The new implementation considers now the silent connections to be when there is no communication in any of both directions.This PR also changes the the silent/drop procedure to be applicable only to the server-side. The
nano::socketclass was changed to get informed on its constructor about theendpoint_typethat can be either socket or client.