Skip to content

Comments

Add lockbud to CI#83

Merged
jking-aus merged 13 commits intosigp:unstablefrom
eserilev:add-lockbud-ci-job
Jan 22, 2025
Merged

Add lockbud to CI#83
jking-aus merged 13 commits intosigp:unstablefrom
eserilev:add-lockbud-ci-job

Conversation

@eserilev
Copy link
Member

@eserilev eserilev commented Jan 7, 2025

Adds lockbud to CI w/ one caveat

The caveat is the CI job never actually fails. Even if a deadlock is found, it'll just log the deadlock in CI, but wont return an error code. We have an issue up in LH here to eventually fix this

So yeah, not the most ideal CI job... but still might be nice to have until we get the job to fail correctly

@eserilev eserilev added the ready-for-review This PR is ready to be reviewed label Jan 7, 2025
@jking-aus jking-aus requested a review from dknopik January 14, 2025 04:47
@magick93
Copy link
Contributor

Even if a deadlock is found, it'll just log the deadlock in CI, but wont return an error code. We have an issue up in LH here to eventually fix this

@eserilev do you have a way to reproduce this - as in some sample rust that will trigger output from lockbud?

If yes, I can try get it to fail the workflow.

@eserilev
Copy link
Member Author

eserilev commented Jan 14, 2025

@magick93

you should be able to force a deadlock with the following snippet
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=89b90214fbcb364bd9ac5ccaf4ff7d01

If its easier you can revert the changes in this commit here in LH:
sigp/lighthouse@101bf81

lockbud will report a deadlock

@dknopik
Copy link
Member

dknopik commented Jan 16, 2025

@magick93, I have introduced such a failure: https://github.com/sigp/anchor/actions/runs/12806469752/job/35705033682?pr=83#step:5:1312 - hope it helps!

Copy link
Member

@dknopik dknopik left a comment

Choose a reason for hiding this comment

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

LGTM

@jking-aus jking-aus merged commit a9c284a into sigp:unstable Jan 22, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review This PR is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants