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

Calling Open on ICommunication leaves async timer running which closes socket. #3895

Closed
peteri opened this issue Sep 10, 2019 · 6 comments
Closed
Assignees
Labels
NET Core 3.1 Release Issues for the NET Core 3.1 release.
Milestone

Comments

@peteri
Copy link

peteri commented Sep 10, 2019

Describe the bug
If Open is called on a netTcp connection (unsure about other types) then if a ServiceContract method is invoked AFTER the open time has expired then the socket will have been closed.

This appears to be due the BeginWriteCore method in SocketConnection.cs starting an timer for asynchronous send operations. The EndWrite method does not cancel the _sendTimer which expires and closes the socket.

Calling a method sends synchronously which calls SetWriteTimeout with synchronous set to true. SetWriteTimeout then clears the _sendTimer by cancelling it.

I believe this is new as the calls to System.Net.NegotiateStream is now async (comparing to the reference source code)

To Reproduce
Add and run the test in this commit (I used Binding.Tcp.IntegrationTests)
peteri@cd6d1f1

Apologies for the "hacky" nature of this as I can't build the current master so tested on the release/2.1.0 branch and the Thread.Sleep.

Expected behavior
Calling Open does not abort the connection.

@peteri peteri changed the title Call Open on ICommunication leaves async timer running which closes socket. Calling Open on ICommunication leaves async timer running which closes socket. Sep 10, 2019
@peteri
Copy link
Author

peteri commented Sep 10, 2019

@StephenBonikowsky StephenBonikowsky added this to the 3.1 milestone Sep 23, 2019
@StephenBonikowsky
Copy link
Member

@peteri Thanks for reporting this! We will target this fix for the next release before the end of the year.

@peteri
Copy link
Author

peteri commented Sep 25, 2019

Sorry about the test being quite so crappy (I hate using Thread.Sleep) but at least it gives you something to work with.

@mconnew
Copy link
Member

mconnew commented Sep 26, 2019

@peteri, I think you can make test methods async and do await Task.Delay(1000) instead.

@StephenBonikowsky
Copy link
Member

Potentially fixed by #3994

@StephenBonikowsky StephenBonikowsky self-assigned this Oct 28, 2019
@StephenBonikowsky
Copy link
Member

The PR has been merged, the change will be part of 3.1 preview3 which will be released sometime in November.

@StephenBonikowsky StephenBonikowsky added the NET Core 3.1 Release Issues for the NET Core 3.1 release. label Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NET Core 3.1 Release Issues for the NET Core 3.1 release.
Projects
None yet
Development

No branches or pull requests

3 participants