search

zeroc-ice / ice

All-in-one solution for creating networked applications with RPC, pub/sub, server deployment, and more.
https://zeroc.com
GNU General Public License v2.0
2.04k stars 592 forks source link

Added support for taking into account message read/write for the inactivity timeout #2222

Closed bentoi closed 3 months ago

bentoi commented 4 months ago

With this PR, a connection which is writing or reading messages other than the connection validation message is not considered inactive.

bentoi commented 3 months ago

I think the logic is too complicated, with 4 calls to cancelInactivityTimerTask (still reasonable) and 10 calls to scheduleInactivityTimerTask (too much). Could we simplify the logic?

Your proposal sounds ok even though it's a bit strange to link the inactivity timer to the idle timeout logic. Another option would be to do like 3.7: update a timestamp each time there's activity and have a timer that regularly checks if there was no activity in the last "inactivity timeout" period.

bernardnormier commented 3 months ago

it's a bit strange to link the inactivity timer to the idle timeout logic.

The logic is when a connection is inactive, the only messages being sent/received are heartbeats. So we piggy-back on the sending of heartbeats: it's when we send a heartbeat that the connection could possibly be detected as inactive.

do like 3.7: update a timestamp each time there's activity and have a timer that regularly checks

Yes, that's another option. The downside is it's an extra timer / thread that locks the connection from time to time, including when the connection is very busy. Whereas when checking during writes, the check is essentially free.

bentoi commented 3 months ago

I'm closing this PR since Bernard's proposal is much simpler than this PR and the current code.