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

Concurrency fix for MacOS #12

Closed
wants to merge 2 commits into from

Conversation

sagacity
Copy link

@sagacity sagacity commented May 15, 2020

This is a fix for #11.

I needed to wrap all NSPasteboard access in this mutex, including the new() fn. I've used a global mutex for this, wrapped in a lazy_static!.

Edit: Hmm, no, apparently even WITH a Mutex around it, it's still failing intermittently :( It appears that even a global Mutex isn't enough, it's simply failing even when multiple pasteboard instances are being created and used. The only way I can get my test to reliably work is if have a shared Arc<Mutex<ClipboardContext>> and re-use that everywhere. Simply creating multiple ClipboardContexts will eventually fail.

@sagacity sagacity force-pushed the osx-concurrency-fix branch 2 times, most recently from a7eaf0b to 72e892f Compare May 15, 2020 10:42
@sagacity sagacity force-pushed the osx-concurrency-fix branch from 72e892f to a780367 Compare May 15, 2020 12:41
@chrisduerr
Copy link
Member

Hmm, no, apparently even WITH a Mutex around it, it's still failing intermittently :( It appears that even a global Mutex isn't enough, it's simply failing even when multiple pasteboard instances are being created and used.

Have you checked Apple's documentation w.r.t. NSPasteboard? I feel like this is an issue that should be documented somewhere and gaining a better understanding of what is actually going on would probably help fix this.

The only way I can get my test to reliably work is if have a shared Arc<Mutex> and re-use that everywhere.

Well in theory if this really is our only option, it should be possible to just always give out a reference to a singleton which is managed with a lock in the background.

@sagacity
Copy link
Author

Let's first do a root cause analysis...

@sagacity sagacity closed this May 15, 2020
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.

2 participants