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

Make concurrent-queue loom-aware #5

Closed
wants to merge 5 commits into from
Closed

Conversation

Restioson
Copy link

@Restioson Restioson commented Sep 11, 2020

This PR implements loom-awareness for concurrent-queue. Particularly, downstream users can pass --cfg loom to rustc via RUSTFLAGS in order to then run loom tests for their crates using concurrent-queue, and have loom be able to test/track the concurrent-queue parts too.

I'm not sure how this interacts with the CI. Ideally, the checking should be run with both loom on and off, the normal tests with loom off, and any loom tests with loom on. In order to enable loom, RUSTFLAGS must be set to have --cfg loom. I haven't written any loom tests for concurrent-queue in this PR yet, though that could come in subsequent work.

@Restioson
Copy link
Author

Ok, it appears that this isn't working yet - not really sure how it didn't trigger in my initial cargo tests - so I will try and fix it.

@Restioson
Copy link
Author

Restioson commented Sep 11, 2020

Okay, I have fixed it. The last unresolved issue is around CI.

@Restioson
Copy link
Author

Restioson commented Sep 12, 2020

There are some workarounds included here which may be possible to get rid of if changes are made in loom (e.g atomic and cell get mut rather than with mut) , so maybe you'd want to wait to see how that pans out?

@Restioson
Copy link
Author

I've been investigating as to why my crate fails under loom, and this could be related to loom not supporting atomic SeqCst fences yet. If this is the case, then this PR may have to be closed pending loom being able to support that. That being said, I'm not sure that is the reason, so it will need some further investigation.

@notgull
Copy link
Member

notgull commented Oct 7, 2022

Obsoleted by #27

@notgull notgull closed this Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants