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

Revert "Display stack trace at stack overflow (#31956)" #32153

Merged
merged 1 commit into from
Feb 12, 2020

Conversation

safern
Copy link
Member

@safern safern commented Feb 12, 2020

This introduced test failures in System.Threading.Tasks:

https://helix.dot.net/api/2019-06-17/jobs/18d7114b-44f4-4b9e-90b5-9ea25e89b4a3/workitems/System.Threading.Tasks.Tests/console

    System.Threading.Tasks.Tests.CESchedulerPairTests.TestCreationOptions(ctorType: "default") [FAIL]
      System.TypeLoadException : Could not load type 'System.Threading.Tasks.SingleProducerSingleConsumerQueue`1+Segment[System.__Canon]' from assembly 'System.Private.CoreLib, Version=5.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e'.
      Stack Trace:
        /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/ProducerConsumerQueues.cs(140,0): at System.Threading.Tasks.SingleProducerSingleConsumerQueue`1..ctor()
        /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/ConcurrentExclusiveSchedulerPair.cs(31,0): at System.Threading.Tasks.ConcurrentExclusiveSchedulerPair..ctor(TaskScheduler taskScheduler, Int32 maxConcurrencyLevel, Int32 maxItemsPerTask)
        /_/src/libraries/System.Threading.Tasks/tests/CESchedulerPairTests.cs(107,0): at System.Threading.Tasks.Tests.CESchedulerPairTests.TestCreationOptions(String ctorType)

Fixes: #32126

cc: @janvorli @jkotas

@jkotas
Copy link
Member

jkotas commented Feb 12, 2020

How did you trace it down? I was trying to reproduce and debug this locally, with not much success.

@janvorli
Copy link
Member

Weird, there is a one line change in the HandleHardwareException for the non-stack overflow path that somehow leaked there from my initial experiments that should not be there and I have no idea what it was supposed to do. That could possibly cause the problem. I'll try to repro the issue locally tomorrow and then see if removing it fixes the problem.

@janvorli
Copy link
Member

Or, would it make sense to not to merge this revert and let me supply a fix early tomorrow instead?

@safern
Copy link
Member Author

safern commented Feb 12, 2020

How did you trace it down? I was trying to reproduce and debug this locally, with not much success.

I looked at the history of changes under the coreclr directory and the first failed build with these failures and then backtracked the commits close to that build. Then started reverting locally until no repro.

@safern
Copy link
Member Author

safern commented Feb 12, 2020

@safern
Copy link
Member Author

safern commented Feb 12, 2020

Or, would it make sense to not to merge this revert and let me supply a fix early tomorrow instead?

I'm OK with that, however, we try to fix the breaks quick as these is bringing our pass rate on CI council down, we were up 20% already on rolling builds, so it would be nice to have this in and probably you can revert this revert and include your fix?

@jkotas
Copy link
Member

jkotas commented Feb 12, 2020

Or, would it make sense to not to merge this revert and let me supply a fix early tomorrow instead?

I am pretty hardcore about using reverts to get the CI fixed ASAP. This crash affected many PRs today.

@safern
Copy link
Member Author

safern commented Feb 12, 2020

I am pretty hardcore about using reverts to get the CI fixed ASAP. This crash affected many PRs today.

Yeah I agree. Plus it affects our CI reliability in telemetry, and it causes pain to people, searching for known issues and linking to those issues, etc. When this is merged, this commit can be reverted and just include an extra commit with the fix.

@safern safern merged commit 90e9614 into dotnet:master Feb 12, 2020
@safern safern deleted the RevertStackTraceStackOverflow branch February 12, 2020 03:11
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.Threading.Tasks.Tests.CESchedulerPairTests failing with TypeLoadException on CoreCLR
4 participants