-
Notifications
You must be signed in to change notification settings - Fork 593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix bugs in idle timeout logic (C#) #3156
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -1941,6 +1941,9 @@ private bool validate(int operation) | |||
throw new MarshalException($"Received ValidateConnection message with unexpected size {size}."); | ||||
} | ||||
TraceUtil.traceRecv(_readStream, this, _logger, _traceLevels); | ||||
|
||||
// Client connection starts sending heartbeats once it's received the ValidateConnection message. | ||||
_idleTimeoutTransceiver?.scheduleHeartbeat(); | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can _idleTimeoutTransceiver ever be null here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's null when the idle timeout is <=0 (i.e., disabled) ice/csharp/src/Ice/ConnectionI.cs Line 1410 in 54548fe
|
||||
} | ||||
} | ||||
|
||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,6 @@ | |
|
||
#nullable enable | ||
|
||
using System.Collections; | ||
using System.Diagnostics; | ||
using System.Net.Sockets; | ||
|
||
|
@@ -61,17 +60,8 @@ public void finishRead(Buffer buf) | |
|
||
public ConnectionInfo getInfo() => _decoratee.getInfo(); | ||
|
||
public int initialize(Buffer readBuffer, Buffer writeBuffer, ref bool hasMoreData) | ||
{ | ||
int op = _decoratee.initialize(readBuffer, writeBuffer, ref hasMoreData); | ||
|
||
if (op == SocketOperation.None) // connected | ||
{ | ||
rescheduleWriteTimer(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug 1: |
||
} | ||
|
||
return op; | ||
} | ||
public int initialize(Buffer readBuffer, Buffer writeBuffer, ref bool hasMoreData) => | ||
_decoratee.initialize(readBuffer, writeBuffer, ref hasMoreData); | ||
|
||
public string protocol() => _decoratee.protocol(); | ||
|
||
|
@@ -108,7 +98,11 @@ public int write(Buffer buf) | |
return op; | ||
} | ||
|
||
internal IdleTimeoutTransceiverDecorator(Transceiver decoratee, ConnectionI connection, TimeSpan idleTimeout, bool enableIdleCheck) | ||
internal IdleTimeoutTransceiverDecorator( | ||
Transceiver decoratee, | ||
ConnectionI connection, | ||
TimeSpan idleTimeout, | ||
bool enableIdleCheck) | ||
{ | ||
Debug.Assert(idleTimeout > TimeSpan.Zero); | ||
|
||
|
@@ -118,9 +112,12 @@ internal IdleTimeoutTransceiverDecorator(Transceiver decoratee, ConnectionI conn | |
if (enableIdleCheck) | ||
{ | ||
_readTimer = new System.Threading.Timer(_ => connection.idleCheck(_idleTimeout)); | ||
} | ||
idleCheckEnabled = enableIdleCheck; | ||
|
||
// idleCheckEnabled remains false for now. ConnectionI calls enableIdleCheck() when it becomes Active to | ||
// schedule the first idle check and set idleCheckEnabled to true. | ||
// We don't want the idle check to start when a client connection is waiting for the initial | ||
// ValidateConnection message or when a server connection is in its initial StateHolding state. | ||
} | ||
_writeTimer = new System.Threading.Timer(_ => connection.sendHeartbeat()); | ||
} | ||
|
||
|
@@ -142,6 +139,8 @@ internal void disableIdleCheck() | |
} | ||
} | ||
|
||
internal void scheduleHeartbeat() => rescheduleWriteTimer(); | ||
|
||
private void cancelReadTimer() => _readTimer!.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); | ||
|
||
private void cancelWriteTimer() => _writeTimer.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even if it doesn't write anything (a rare situation).
On the server side, the writing of the initial ValidateConnection message schedules the first heartbeat.