Skip to content
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

Graceful termination of socket connections to the RTI #302

Merged
merged 2 commits into from
Aug 23, 2024

Conversation

byeonggiljun
Copy link
Collaborator

No description provided.

Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you been able to confirm that this addresses the non-deterministic failure we've been seeing?

@byeonggiljun
Copy link
Collaborator Author

byeonggiljun commented Aug 23, 2024

@lhstrh As a federate destroys the socket connection right after sending LTC and Resign signals, the RTI potentially misses those signals. Thus, I propose using socket.end() instead of socket.destroy() so that the buffer is flushed before terminating. However, as both socket.write() and socket.end() are asynchronous, I guess socket.end() may close the connection before socket.write() writes any message to the buffer.

protected _finish(): void {
this.sendRTILogicalTimeComplete(this.util.getCurrentTag());
this.sendRTIResign();
this.shutdownRTIClient();
super._finish();
}

To completely mitigate this, we have to manage a buffer ourselves to track pending network messages that need to be sent. But this PR makes it much better than master and works fine in most cases.

@lhstrh
Copy link
Member

lhstrh commented Aug 23, 2024

Cool! It might make sense to document your analysis in an issue, should the problem resurface.

@byeonggiljun byeonggiljun added federated enhancement Enhancement of existing feature labels Aug 23, 2024
@byeonggiljun
Copy link
Collaborator Author

Cool! It might make sense to document your analysis in an issue, should the problem resurface.

I created the issue #303!

@byeonggiljun byeonggiljun added bugfix and removed enhancement Enhancement of existing feature labels Aug 23, 2024
@byeonggiljun byeonggiljun merged commit f5df4fa into master Aug 23, 2024
4 checks passed
@byeonggiljun byeonggiljun deleted the socket-termination branch August 23, 2024 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants