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

Revert the reimplementation of TBQueue #78

Closed
nikita-volkov opened this issue Oct 20, 2023 · 7 comments
Closed

Revert the reimplementation of TBQueue #78

nikita-volkov opened this issue Oct 20, 2023 · 7 comments

Comments

@nikita-volkov
Copy link
Contributor

I've managed to reproduce one issue in #76 and provide a fix for it in #77. However I'm experiencing other race conditions on 2.5.2.1, which were not present on 2.5.1.0. It's hard to reproduce though.

Any way, it means that there is at least two bugs in the reimplementation and it has no test coverage. So there may be more.

Until the new implementation is covered with exhaustive tests and is fixed I suggest to roll back and deprecate the 2.5.2.0 and 2.5.2.1 releases.

Regarding an exhaustive test model for the new implementation I've made a suggestion here: #76 (comment).

@konsumlamm
Copy link
Contributor

I'd recommend adding property tests (e.g. using QuickCheck). While we're at it, we can also replace test-framework by tasty (which is more actively developed).

Currently there are two test suites, one in tests/ and one in testsuite/. It seems tests/ is just for GHC? It would be nice to unify the two.

@TeofilC
Copy link

TeofilC commented Oct 23, 2023

Another way forward might be to start off with the new implementation as a TBQueue.Experimental variant or something like that and only replace the old version with it after a while has passed and we've worked out the kinks

@simonmar
Copy link
Member

simonmar commented Nov 2, 2023

We should probably roll it back until we have better testing. @bgamari would you be able to do that?

@bgamari
Copy link
Contributor

bgamari commented Nov 16, 2023

Yes, I can roll-back.

FWIW, I agree that we need better testing; it has long been concerning to me that such a subtle system as STM has as little testing infrastructure as it does.

bgamari added a commit that referenced this issue Nov 16, 2023
This reverts commit 9821578.

As noted in #78, the current implementation does not appear to be correct.
bgamari added a commit that referenced this issue Nov 17, 2023
This reverts commit 9821578.

As noted in #78, the current implementation does not appear to be correct.
bgamari added a commit that referenced this issue Nov 17, 2023
This reverts commit 9821578.

As noted in #78, the current implementation does not appear to be correct.
@bgamari
Copy link
Contributor

bgamari commented Nov 17, 2023

Reverted in #80.

@bgamari
Copy link
Contributor

bgamari commented Nov 17, 2023

I have released 0.5.3.0 with the TBQueue rewrite reverted.

@konsumlamm , perhaps you would like to have a stab at improving stm's testsuite and then trying your change again?

@bgamari bgamari closed this as completed Nov 17, 2023
@nikita-volkov
Copy link
Contributor Author

nikita-volkov commented Nov 17, 2023

Thank you! Can we also deprecate 2.5.2.0, 2.5.2.1 on Hackage since they are buggy?

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

No branches or pull requests

5 participants