Document thread safety...

[icculus]

This is more of a wishlist item than a hard requirement, mostly because this job might suck, but it might be nice if we added a line to the documentation of each API function that detailed thread safety:

\threadsafety This function can be called at any time.

or

\threadsafety This function has undefined behavior if not called on the main thread, including crashing your program; ***this is a platform-specific problem, not an SDL bug***.

or

\threadsafety This function should not be called from multiple threads at once, please use a mutex.

or

\threadsafety This function can conflict with SDL_SomeThingElse when called from multiple threads at once.

etc.

And then adjust the wiki bridge to know to add highlighting to that tag, like it does for \since.

[slouken]

Let's go ahead and add the infrastructure for it, and then markup the functions most frequently asked about.

[icculus]

Taking this to do the wiki bridge work real fast.

[icculus]

And just discovered the bridge hasn't been working since we moved everything into include/SDL3. :/

[icculus]

Ok, the wiki bridge is fixed up, and \threadsafety can be added to functions now. I added a way-too-verbose one to SDL_RenderPresent just to test this. I'll make that less wordy and then I'm unassigning myself for now, if someone else wants to take this on.

[madebr]

I'm seeing this on a few workflows:

 FAILED: CMakeFiles/SDL3_test.dir/src/test/SDL_test_assert.c.o 
/opt/hostedtoolcache/ndk/r21e/x64/toolchains/llvm/prebuilt/linux-x86_64/bin/clang --target=aarch64-none-linux-android23 --gcc-toolchain=/opt/hostedtoolcache/ndk/r21e/x64/toolchains/llvm/prebuilt/linux-x86_64 --sysroot=/opt/hostedtoolcache/ndk/r21e/x64/toolchains/llvm/prebuilt/linux-x86_64/sysroot  -I/home/runner/work/SDL/SDL/build/include-config-release -I/home/runner/work/SDL/SDL/build/include -I/home/runner/work/SDL/SDL/include -I/home/runner/work/SDL/SDL/include/SDL3 -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security  -O2 -DNDEBUG  -fPIC -idirafter /home/runner/work/SDL/SDL/src/video/khronos -Wall -fno-strict-aliasing -Werror=declaration-after-statement -Wdeclaration-after-statement -Werror=documentation -Wdocumentation -Werror=documentation-unknown-command -Wdocumentation-unknown-command -Wshadow -fcolor-diagnostics -Werror -MD -MT CMakeFiles/SDL3_test.dir/src/test/SDL_test_assert.c.o -MF CMakeFiles/SDL3_test.dir/src/test/SDL_test_assert.c.o.d -o CMakeFiles/SDL3_test.dir/src/test/SDL_test_assert.c.o -c /home/runner/work/SDL/SDL/src/test/SDL_test_assert.c
In file included from /home/runner/work/SDL/SDL/src/test/SDL_test_assert.c:27:
In file included from /home/runner/work/SDL/SDL/include/SDL3/SDL_test.h:33:
In file included from /home/runner/work/SDL/SDL/include/SDL3/SDL.h:66:
/home/runner/work/SDL/SDL/include/SDL3/SDL_render.h:1396:4: error: unknown command tag name [-Werror,-Wdocumentation-unknown-command]
 * \threadsafety You may only call this function on the main thread. If this
   ^~~~~~~~~~~~~
1 error generated.

Perhaps we should use a different escape character? #threadsafety?

If we need to stick to doxygen commands, what about the following?

\par Thread safety
You may only call this function on the main thread.

[icculus]

We do not need to stick to Doxygen commands, and we should turn off any compiler feature that assumes we do.

[madebr]

The cmake code enabling checking for valid documentation was added to the CMake script 3 weeks ago

https://github.com/libsdl-org/SDL/blob/a37f2aed7e9b53a7e09a6f619344ca6afdc63881/CMakeLists.txt#L593-L602

[slouken]

I removed the problematic warnings.

[LunarLambda]

Is there a rough list of what definitely is and definitely isn't thread safe? I'm working on Rust bindings for the upcoming SDL 3.0.0 release and one thing that has been a major issue for previous work on SDL2 was people getting the thread safety (codified in the language) requirements wrong.

Roughly, I know Video and Events are strictly main-thread only, I've heard that Audio and Timers might be thread-safe (appears to be already documented in my checkout)

A breakdown by-header would be excellent (even if most of it ends up being "unsure")

[icculus]

Yeah, obtaining that information is the whole task here. :)

That being said, it might be useful if someone wrote a script to add...

\threadsafety Unknown. Please update this documentation!

...to everything that didn't have this tag yet, so we can just grep for things that still need to be documented.

[LunarLambda]

not a bad idea, maybe

another thing I'd like clarification on is the meaning of "main thread"

is that the thread that called SDL_Init or the thread that is running main()? Asking specifically because of SDL_RunApp, whether that imposes any further restrictions

[Lokathor]

On mac at least I believe it's the actual main() thread, which means for proper cross-platform you would always have to only use that thread.

[icculus]

Almost always it means "the thread that literally runs main()" ...the first thread of the program.

This is a hard requirement in several platforms (Emscripten has a ton of requirements that crucial things happen in only that thread regardless of where SDL_Init is called, other platforms expect that thread to be where the GUI is initialized, etc)

[LunarLambda]

good to know, thank you

[icculus]

There are currently 743 functions missing thread safety info:

https://gist.github.com/icculus/3545d69c12bb160b99fdb05d6c5af34f

This is almost certainly going to slip out of 3.2.0, but I'll try to knock this down a little as it goes along.

[icculus]

Screw it, I set an alert on my phone to do at least 25 of these a day, so we can be done in about 30 days, only spending a few minutes a day on it.

Once I run out of low-hanging fruit, I might stop, though.