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

Redisable GetAndSet_AreThreadSafe_AndUpdatesNeverLeavesNullValues test #72882

Merged
merged 2 commits into from
Jul 27, 2022

Conversation

danmoseley
Copy link
Member

Relates to #72879

Seems to be only reproing on Arm, and if that's the case would explain why I could not repro it.

@danmoseley
Copy link
Member Author

danmoseley commented Jul 26, 2022

As to why -- my first thought was that the write to readValueIsNull needs to be volatile. But the assert is that it's false. (Plus, @stephentoub pointed out that the write cannot move past IsCancellationRequested as it does a volatile read, and the read cannot move before WaitAny as it performs a memory barrier).

There is likely a memory incorrectness issue in the cache.

Test was originally modified to the current form here: aspnet/Caching@21c850a#diff-2b1f7c7cf3a8b8b4966f4a7fe85e10172f283fb96af85693cc7d30f923ee7dbaR444 in case that's relevant next time this is looked at.

@danmoseley
Copy link
Member Author

danmoseley commented Jul 26, 2022

System.AggregateException : One or more errors occurred.
---- System.Threading.Tasks.TaskCanceledException : A task was canceled.


Stack trace
   at System.Threading.Tasks.Task.WaitAll(Task[] tasks, Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at Microsoft.Extensions.Caching.Memory.MemoryCacheSetAndRemoveTests.OvercapacityPurge_AreThreadSafe() in /_/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs:line 655
----- Inner Stack Trace -----

I'll open a bug for that, and disable those tests again as well. They need rewriting. I just exposed this issue.

edit: disabled against #72890

@danmoseley danmoseley merged commit f90028c into dotnet:main Jul 27, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants