osu-framework icon indicating copy to clipboard operation
osu-framework copied to clipboard

SDL clipboard implementation is potentially unsafe

Open hwsmm opened this issue 10 months ago • 3 comments

While investigating https://github.com/ppy/osu/issues/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?

hwsmm avatar Feb 24 '25 15:02 hwsmm

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.

Susko3 avatar Feb 24 '25 20:02 Susko3

https://github.com/libsdl-org/SDL/commit/bc4185c685e0db472ff0884d4adf2239dc046c32 seems to indicate this should only be an issue on apple.

bdach avatar Feb 25 '25 07:02 bdach

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

hwsmm avatar Feb 25 '25 07:02 hwsmm