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

Tighten down locking on datetime tests #866

Closed
wants to merge 1 commit into from

Conversation

davidhewitt
Copy link
Member

After the datetime tests failed again on my machine, I realised that lock!() needs to go before the acquire_gil() call - otherwise released objects can still go into the "wrong" GILPool.

@programmerjake
Copy link
Contributor

From what I can see, the lock!() macro effectively does nothing since each call creates a new mutex then locks it, every function gets it's own new mutex at every call so none of them are shared. what you probably wanted was to lock a global mutex since that may actually do something.

@programmerjake
Copy link
Contributor

The Travis CI failure is caused by parking_lot being updated for the renamed asm -> llvm_asm macro but the nightly compiler being too old to have the macro renamed yet.

@programmerjake
Copy link
Contributor

see rust-lang/rust#68404

@davidhewitt
Copy link
Member Author

Yikes, sorry, you're right about the Mutex, I should have caught that in the original PR. I'll fix this PR and also create one to update the minimum nightly in the morning.

@kngwyu
Copy link
Member

kngwyu commented Apr 11, 2020

From what I can see, the lock!() macro effectively does nothing since each call creates a new mutex then locks it, every function gets it's own new mutex at every call so none of them are shared.

Ah, sorry about that.
Could I open a PR that fixes it myself?

@davidhewitt
Copy link
Member Author

Closed in favour of #867

@davidhewitt davidhewitt deleted the better-datetime-lock branch August 10, 2021 07:19
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

Successfully merging this pull request may close these issues.

3 participants