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

Fix #4083 - Endpoint receive buffer stack overflow #4089

Merged
merged 2 commits into from Dec 12, 2019

Conversation

@nagytech
Copy link
Contributor

nagytech commented Dec 11, 2019

The ordering of the 2nd and 3rd parameters for C#'s ConcurrentDictionary.TryUpdate are inverted from Scala's Map.replace.

public bool TryUpdate (TKey key, TValue newValue, TValue comparisonValue); docs

vs

abstract def replace(k: A, oldvalue: B, newvalue: B): Boolean docs

A quick review of the entire Akka.NET code base only shows a few usages of TryUpdate. One in particular appears to have the old and new parameters backwards.

(Scala implementation for reference)
https://github.com/akka/akka/blob/edaea382addc5831ca28beed504d879fcf01cd1a/akka-remote/src/main/scala/akka/remote/Endpoint.scala#L1111-L1114

This appears to be causing #4083

nagytech and others added 2 commits Dec 11, 2019
Copy link
Member

Aaronontheweb left a comment

LGTM - yep, it appears this was a mis-translation due to the C# and Scala APIs being reversed.

@Aaronontheweb Aaronontheweb merged commit d228f94 into akkadotnet:dev Dec 12, 2019
8 checks passed
8 checks passed
PR Validation #2019.12.12.7 succeeded
Details
PR Validation (.NET Core Multi-Node Tests (Windows)) .NET Core Multi-Node Tests (Windows) succeeded
Details
PR Validation (.NET Core Unit Tests (Linux)) .NET Core Unit Tests (Linux) succeeded
Details
PR Validation (.NET Core Unit Tests (Windows)) .NET Core Unit Tests (Windows) succeeded
Details
PR Validation (.NET Framework Multi-Node Tests (Windows)) .NET Framework Multi-Node Tests (Windows) succeeded
Details
PR Validation (.NET Framework Unit Tests (Windows)) .NET Framework Unit Tests (Windows) succeeded
Details
PR Validation (NuGet Pack) NuGet Pack succeeded
Details
PR Validation (Windows Build) Windows Build succeeded
Details
Aaronontheweb added a commit to Aaronontheweb/akka.net that referenced this pull request Dec 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.