[Async TestKit] Modernize TcpStage and TcpListener #5989
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ocking by running them in their own thread
Arkatufus
changed the title
| public static WritingResumed Instance = new WritingResumed(); | ||
| public static readonly WritingResumed Instance = new WritingResumed(); | ||
|
|
||
| private WritingResumed() |
There was a problem hiding this comment.
These are breaking API changes, but no big deal. Need to be able to do stuff like this in Akka.NET v1.5.
| try | ||
| { | ||
| bind.Options.ForEach(x => x.BeforeServerSocketBind(_socket)); | ||
| _socket.Bind(bind.LocalAddress); |
There was a problem hiding this comment.
Good idea, moving this out of the constructor
| _binding = true; | ||
| _bind = bind; | ||
| _acceptLimit = bind.PullMode ? 0 : _tcp.Settings.BatchAcceptLimit; | ||
| BindAsync().PipeTo(Self); |
| if (!_socket.AcceptAsync(saea)) | ||
| Self.Tell(new SocketEvent(saea)); | ||
| return true; | ||
|
|
||
| case Tcp.ResumeAccepting resumeAccepting: | ||
| _acceptLimit = resumeAccepting.BatchSize; | ||
| // TODO: this is dangerous, previous async args are not disposed and there's no guarantee that they're not still receiving data |
There was a problem hiding this comment.
Previous socket async event args?
There was a problem hiding this comment.
_saeas is an array of SocketAsyncEventArgs, assigned by the Accept method.
In this case, the old code actually discarded all of the old args while most of them still active accepting connections, we would lose all reference to those event args.
- They cause a memory leak, or
- They can get finalized and disposed and something breaks.
There was a problem hiding this comment.
aren't these SAEA only handling bind events though, which have no buffers?
Aaronontheweb
approved these changes
Jun 10, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes:
Make TcpListener binding and unbinding code non-blocking by running them on another thread.