This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Attempt at simplfying this (SDL-createthread-simplify-RYAN1.diff, text/plain, 2020-03-27 02:40:30 +0000, 11502 bytes)

Reported in version: HG 2.0
Reported for operating system, platform: Other, All

Comments on the original bug report:

On 2020-03-27 01:42:59 +0000, Ryan C. Gordon wrote:

So it looks like from here:

• App calls SDL_CreateThread*
• SDL_CreateThread allocates some data, creates a semaphore, calls into SDL_SYS_CreateThread
• SDL_SYS_CreateThread spins a thread and calls SDL_RunThread on the new thread, and returns to SDL_CreateThread*
• SDL_CreateThread waits on the semaphore.
• SDL_RunThread sets up some stuff, posts to the semaphore, and calls the app's actual thread entry point.
• SDL_CreateThread sees the semaphore has posted, takes this as an indication that it's safe to free the data it allocated.

...is there any reason we can't do this:

• Don't create a semaphore at all.
• SDL_RunThread frees the data before calling the app's entry point.
• SDL_CreateThread frees the data if SDL_SYS_CreateThread fails, otherwise it leaves it alone.

This means SDL won't have to create a semaphore and won't have to wait for the new thread at all.

The other benefit: Emscripten with pthread support doesn't actually create the thread until the next iteration through the mainloop, so it can't safely wait here by default, as the thread will never post to the semaphore because the thread won't be created at all until some time after SDL_CreateThread returns.

All this to say: is this a safe change to make, or was the semaphore approach done for a specific reason?

--ryan.

On 2020-03-27 01:48:11 +0000, Ryan C. Gordon wrote:

...or just make "data" and "func" from thread_args part of SDL_Thread, and don't free anything extra at all...

--ryan.

On 2020-03-27 02:40:30 +0000, Ryan C. Gordon wrote:

Created attachment 4281
Attempt at simplfying this

Here's a patch to clean it up. Unless it was super important, this nukes a bunch of code, some memory management and a semaphore, and should fix Emscripten threads too.

--ryan.

On 2020-03-27 22:13:58 +0000, Sam Lantinga wrote:

Looks good!
https://hg.libsdl.org/SDL/rev/498a7f27758e