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

SDL clipboard implementation is potentially unsafe #6538

Open
hwsmm opened this issue Feb 24, 2025 · 3 comments
Open

SDL clipboard implementation is potentially unsafe #6538

hwsmm opened this issue Feb 24, 2025 · 3 comments

Comments

@hwsmm
Copy link
Contributor

hwsmm commented Feb 24, 2025

While investigating ppy/osu#32038 (I couldn't reproduce, but anyway), I found that our SDL clipboard implementation may be unsafe.

SDL_clipboard.h requires all functions to run on the main thread, but we run these on a thread that called Set/Get methods.

I could probably try fixing it myself, but it is a bit tricky. If Get methods have to run on the main thread, do we need to wait for thread sleep?

@Susko3
Copy link
Member

Susko3 commented Feb 24, 2025

I would make the Get{Text,Image} methods of the Clipboard API async, then schedule the SDL clipboard APIs calls on the main thread and report the result via the usual task mechanisms. Have a look at AudioComponent for inspiration.

-public abstract string? GetText();
+public abstract Task<string?> GetText();

The textbox api usage would be:

clipboard.GetText().ContinueWith(t => Schedule(insertString, t.GetResultSafely()));

The set methods are already fire-and-forget, so they can just be scheduled on the main thread without making breaking changes to the API.

@bdach
Copy link
Collaborator

bdach commented Feb 25, 2025

libsdl-org/SDL@bc4185c seems to indicate this should only be an issue on apple.

@hwsmm
Copy link
Contributor Author

hwsmm commented Feb 25, 2025

I forgot to update the OP, but I managed to reproduce the issue. I'll try fixing it before reporting this to upstream.

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

3 participants