Skip to content

Fix unsafe usage of Base.Condition #234

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

Merged
merged 5 commits into from
Jul 17, 2021
Merged

Conversation

jpsamaroo
Copy link
Member

No description provided.

@jpsamaroo
Copy link
Member Author

@krynju @ExpandingMan please give this a try

@jpsamaroo jpsamaroo linked an issue Jul 17, 2021 that may be closed by this pull request
@jpsamaroo jpsamaroo force-pushed the jps/thread-safe-notify branch from d324987 to a7a06b9 Compare July 17, 2021 18:59
@jpsamaroo jpsamaroo merged commit 2ed91b7 into master Jul 17, 2021
@jpsamaroo jpsamaroo deleted the jps/thread-safe-notify branch July 17, 2021 18:59
@krynju
Copy link
Member

krynju commented Jul 17, 2021

Works fine for me!
Also checked it with #223 merged and works well together

@ExpandingMan
Copy link
Contributor

Looks good to me as well, thanks for the quick fix.

Btw, I'm pretty sure this problem exists on your recent tag, so a new version tag is likely in order.

Also, this is making me nervous about tests, which don't seem to have been running. Is your intention for master to always be passing? We should probably figure out why tests aren't running. If you'd like I can do a PR to run github workflow tests, though I'm not sure how nicely they play with multiple processes.

@jpsamaroo
Copy link
Member Author

Btw, I'm pretty sure this problem exists on your recent tag, so a new version tag is likely in order.

I just submitted 0.11.5, if that's what you mean.

Also, this is making me nervous about tests, which don't seem to have been running.

The Buildkite tests are the most important, and they're generally passing (with some stochastic failures on nightly that I don't have the bandwidth to debug right now). Windows is just very unreliable with Dagger testing, but it is good to have a sense for how well things are working for those users.

Is your intention for master to always be passing?

If possible, yes, but I can tolerate some failures due to random chance. Much of the unreliability in CI we see appears to be due to Distributed, which I haven't had time to sit down and figure out.

@ExpandingMan
Copy link
Contributor

Maybe this is a stupid question but are the buildkite tests visible somewhere? I don't think I'm seeing them in the PR's...

@jpsamaroo
Copy link
Member Author

Oh, shoot, you're supposed to be able to see them. @DilumAluthge any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Concurrency violation detected on multithread config
3 participants