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

CritSection runaway with windows.wt missing on linux #1249

Closed
baconpaul opened this issue Oct 15, 2019 · 7 comments
Closed

CritSection runaway with windows.wt missing on linux #1249

baconpaul opened this issue Oct 15, 2019 · 7 comments
Assignees
Labels
Linux Issues which only occur on Linux

Comments

@baconpaul
Copy link
Collaborator

As reported by @codecollider in #1247 if you run surge on linux with the shared assets missing, you get a runaway in the critical section recursion. That's interesting and we should debug it one day, even though the proximate problem in #1247 was solved with better error reporting.

@baconpaul baconpaul added this to the Currently Unscheduled milestone Oct 15, 2019
@baconpaul baconpaul modified the milestones: Currently Unscheduled, 1.6.6 Feb 1, 2020
@baconpaul baconpaul added the Linux Issues which only occur on Linux label Feb 1, 2020
@baconpaul baconpaul modified the milestones: Currently Unscheduled, 1.8.0 Oct 2, 2020
@mvf mvf self-assigned this Oct 2, 2020
@mkruselj
Copy link
Collaborator

mkruselj commented Oct 6, 2020

@mvf since CriticalSection is now removed and replaced with std::mutex, is this issue still relevant?

@baconpaul
Copy link
Collaborator Author

It is. In fact if this happens we will deadlock instead of run so we need to understand why it happened before 18 or change the mutex to recursive which sucks

@mvf
Copy link
Collaborator

mvf commented Oct 6, 2020

Yeah, would love to get to the bottom of this. Spent quite some time already trying to reproduce this with the current version by deleting windows.wt with 8f4e694 reverted, then trying to load "Window" into the oscillator. Couldn't get it to deadlock/recurse yet but will poke around some more.

If we can't figure this out, this old issue alone wouldn't be reason enough for me personally to consider recursive mutex. What was once a bad code path should now be barred, and to the user both an endless loop and a deadlock manifest as a hang. So even if this strikes again it's not really a regression IMHO.

@baconpaul
Copy link
Collaborator Author

I agree if we can't repro it we should go ahead
I also agree we should poke at it some more
I also agree we have time
So lets leave this open and we can poke at it some
:)

@mkruselj
Copy link
Collaborator

mkruselj commented Oct 7, 2020

Here's a thought. Why don't we actually subsume windows.wt into the binary itself? Then it's always gonna be available, no way to deadlock in that case?

@baconpaul
Copy link
Collaborator Author

while that would solve the proximate problem
i want to understand why it recursed
in case it would in other failure cases

@baconpaul
Copy link
Collaborator Author

So I just tried for another half hour to make this happen again
I am unable to
I conclude that fixes we put in other parts of the codes to pthis runaway which was hard to reproduce anyway
So I'm going to close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linux Issues which only occur on Linux
Projects
None yet
Development

No branches or pull requests

3 participants