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

Safer thread parkers #110

Merged
merged 9 commits into from
Dec 7, 2018
Merged

Safer thread parkers #110

merged 9 commits into from
Dec 7, 2018

Conversation

faern
Copy link
Collaborator

@faern faern commented Dec 6, 2018

Continuing the work of trying to remove unsafe from methods/functions that actually can't result in UB.

I know the spin based generic thread parker is also in the other PR (#106). But it was kind of needed here to get rid of unsafe prepare_unpark on (almost) all platforms. I will remove that commit from the other PR if/when this is merged.

I left the two constructors of the Windows thread parkers as unsafe. The documentation for GetModuleHandleA suggests it has to be carefully called and used in a multi threaded environment. At the same time I'm not sure what exactly could cause it to fail, or if that is even possible in our setup. If you have more information on the topic, maybe we can safely mark them as safe?

@faern faern force-pushed the safer-thread-parkers branch 2 times, most recently from 8808a37 to c53cb2f Compare December 7, 2018 00:06
@Amanieu
Copy link
Owner

Amanieu commented Dec 7, 2018

The warnings about GetModuleHandle can be safely ignored I think. This only applies to DLLs that might be dynamically unloaded while we are still using them. This shouldn't apply to core system DLLs like ntdll.dll or api-ms-win-core-synch-l1-2-0.dll. However if you want to be absolutely sure then you can use LoadLibraryA instead, which is slower (but it should still be fine considering this is one-time initialization).

@faern faern force-pushed the safer-thread-parkers branch from c53cb2f to dcd6019 Compare December 7, 2018 00:17
@faern faern force-pushed the safer-thread-parkers branch from dcd6019 to e3da313 Compare December 7, 2018 00:26
@faern faern force-pushed the safer-thread-parkers branch from e3da313 to a70199b Compare December 7, 2018 00:34
@Amanieu Amanieu merged commit 604d343 into Amanieu:master Dec 7, 2018
@faern faern deleted the safer-thread-parkers branch December 7, 2018 08:14
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.

2 participants