Skip to content

Conversation

michaelsproul
Copy link
Member

@michaelsproul michaelsproul commented Oct 12, 2023

Issue Addressed

Fix a deadlock in the tests that was causing tests on tree-states to run for hours without finishing: https://github.com/sigp/lighthouse/actions/runs/6491194654/job/17628138360.

Proposed Changes

Avoid using a Mutex under the Rayon par_iter. Instead, use an AtomicUsize. I've run the new version several times in a loop and it hasn't deadlocked (it was deadlocking consistently on tree-states).

Additional Info

The same bug exists in unstable and tree-states, but I'm not sure why it was triggering so consistently on the tree-states branch.

@michaelsproul michaelsproul added test improvement Improve tests ready-for-review The code is ready for review low-hanging-fruit Easy to resolve, get it before someone else does! labels Oct 12, 2023
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Oct 13, 2023
@jimmygchen
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Oct 18, 2023
## Issue Addressed

Fix a deadlock in the tests that was causing tests on tree-states to run for hours without finishing: https://github.com/sigp/lighthouse/actions/runs/6491194654/job/17628138360.

## Proposed Changes

Avoid using a Mutex under the Rayon `par_iter`. Instead, use an `AtomicUsize`. I've run the new version several times in a loop and it hasn't deadlocked (it was deadlocking consistently on tree-states).

## Additional Info

The same bug exists in unstable and tree-states, but I'm not sure why it was triggering so consistently on the tree-states branch.
@jimmygchen
Copy link
Member

bors cancel

@bors
Copy link

bors bot commented Oct 18, 2023

Canceled.

@jimmygchen
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Oct 18, 2023
## Issue Addressed

Fix a deadlock in the tests that was causing tests on tree-states to run for hours without finishing: https://github.com/sigp/lighthouse/actions/runs/6491194654/job/17628138360.

## Proposed Changes

Avoid using a Mutex under the Rayon `par_iter`. Instead, use an `AtomicUsize`. I've run the new version several times in a loop and it hasn't deadlocked (it was deadlocking consistently on tree-states).

## Additional Info

The same bug exists in unstable and tree-states, but I'm not sure why it was triggering so consistently on the tree-states branch.
@bors
Copy link

bors bot commented Oct 18, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Oct 18, 2023
## Issue Addressed

Fix a deadlock in the tests that was causing tests on tree-states to run for hours without finishing: https://github.com/sigp/lighthouse/actions/runs/6491194654/job/17628138360.

## Proposed Changes

Avoid using a Mutex under the Rayon `par_iter`. Instead, use an `AtomicUsize`. I've run the new version several times in a loop and it hasn't deadlocked (it was deadlocking consistently on tree-states).

## Additional Info

The same bug exists in unstable and tree-states, but I'm not sure why it was triggering so consistently on the tree-states branch.
@jimmygchen
Copy link
Member

bors r+

@bors
Copy link

bors bot commented Oct 18, 2023

Already running a review

@bors
Copy link

bors bot commented Oct 18, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Oct 18, 2023
## Issue Addressed

Fix a deadlock in the tests that was causing tests on tree-states to run for hours without finishing: https://github.com/sigp/lighthouse/actions/runs/6491194654/job/17628138360.

## Proposed Changes

Avoid using a Mutex under the Rayon `par_iter`. Instead, use an `AtomicUsize`. I've run the new version several times in a loop and it hasn't deadlocked (it was deadlocking consistently on tree-states).

## Additional Info

The same bug exists in unstable and tree-states, but I'm not sure why it was triggering so consistently on the tree-states branch.
@bors
Copy link

bors bot commented Oct 18, 2023

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title Fix Rayon deadlock in test utils [Merged by Bors] - Fix Rayon deadlock in test utils Oct 18, 2023
@bors bors bot closed this Oct 18, 2023
Gua00va pushed a commit to Gua00va/lighthouse that referenced this pull request Oct 18, 2023
## Issue Addressed

Fix a deadlock in the tests that was causing tests on tree-states to run for hours without finishing: https://github.com/sigp/lighthouse/actions/runs/6491194654/job/17628138360.

## Proposed Changes

Avoid using a Mutex under the Rayon `par_iter`. Instead, use an `AtomicUsize`. I've run the new version several times in a loop and it hasn't deadlocked (it was deadlocking consistently on tree-states).

## Additional Info

The same bug exists in unstable and tree-states, but I'm not sure why it was triggering so consistently on the tree-states branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low-hanging-fruit Easy to resolve, get it before someone else does! ready-for-merge This PR is ready to merge. test improvement Improve tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants