Skip to content

Replace non-reentrant C stdlib calls with thread-safe alternatives#25594

Open
capocasa wants to merge 2 commits intonim-lang:develfrom
capocasa:devel2
Open

Replace non-reentrant C stdlib calls with thread-safe alternatives#25594
capocasa wants to merge 2 commits intonim-lang:develfrom
capocasa:devel2

Conversation

@capocasa
Copy link
Copy Markdown
Collaborator

@capocasa capocasa commented Mar 9, 2026

Summary

During pharao development I noticed some theoretical and reproducible non-thread-safe C calls in the Nim stdlib. Here is a list of C functions known not to be thread safe. I could reproduce data corruption on some of them for threaded access, while others seem to be thread safe in glibc but possibly not in other C libs. Performance should be equivalent.

Module Non-reentrant Replacement
times localtime localtime_s (MSVC) / localtime_r (MinGW, POSIX)
oserrors, syncio, osproc strerror strerror_r (via shared helper)
nativesockets getprotobyname getprotobyname_r
nativesockets getservbyname getservbyname_r
nativesockets getservbyport getservbyport_r
nativesockets gethostbyname getaddrinfo
nativesockets gethostbyaddr getnameinfo

strerror_r uses a bit of C code to detect a glibc function signature special case. An explicit tzset is added to localtime_r instead of localtime's implicit one.

This is a bit more change than I'd hope for but I'm not sure how to be more concise about it. Questions welcome.

@Araq
Copy link
Copy Markdown
Member

Araq commented Mar 9, 2026

The only thing that matters if the glibc / the OS's libc isn't threadsafe, in reality, in 2026. "Known not to be threadsafe on SunOS in 1998" -- I couldn't care less.

@capocasa capocasa force-pushed the devel2 branch 2 times, most recently from 4443b12 to 5888efa Compare March 10, 2026 11:24
@capocasa
Copy link
Copy Markdown
Collaborator Author

Okay so first off that patch was way too invasive, it's clean now.

Here is a test program and is results:

nim e run.nims
Hint: used config file '/home/carlo/.choosenim/toolchains/nim-2.2.8/config/nim.cfg' [Conf]
Hint: used config file '/home/carlo/.choosenim/toolchains/nim-2.2.8/config/config.nims' [Conf]
Hint: used config file '/home/carlo/.config/nim/nim.cfg' [Conf]
Hint: used config file '/home/carlo/.config/nim/config.nims' [Conf]
  getProtoByName (glibc)
  getProtoByName (musl)
  getServByName (glibc)
  getServByName (musl)
  getServByPort (glibc)
  getServByPort (musl)
  getHostByName (glibc)
  getHostByName (musl)
  localtime (glibc)
  localtime (musl)

Thread-safety corruption test (8 threads, 50k iterations each)

Function          glibc                  musl
----------------- ---------------------- -------------------
getProtoByName    30/400000 CORRUPTED    ERROR
getServByName     6/40000 CORRUPTED      1/40000 CORRUPTED
getServByPort     1/8000 CORRUPTED       93/8000 CORRUPTED
getHostByName     SIGSEGV                SIGSEGV
localtime         524/400000 CORRUPTED   18/400000 CORRUPTED

@capocasa
Copy link
Copy Markdown
Collaborator Author

TLDR; the test program proves that 2026 glibc and musl are not, in fact, thread safe in the way the Nim stdlib is currently calling them.

@Araq
Copy link
Copy Markdown
Member

Araq commented Mar 13, 2026

Hint: used config file 'D:\a\Nim\Nim\compiler\nim.cfg' [Conf]
d:/a/nim/nim/dist/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.1.0/../../../../x86_64-w64-mingw32/bin/ld.exe: D:/a/Nim/Nim/nimcache/r_windows_amd64/@ptimes.nim.c.o:@ptimes.nim.c:(.text+0x40df): undefined reference to localtime_r' d:/a/nim/nim/dist/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.1.0/../../../../x86_64-w64-mingw32/bin/ld.exe: D:/a/Nim/Nim/nimcache/r_windows_amd64/@ptimes.nim.c.o:@ptimes.nim.c:(.text+0x4166): undefined reference to localtime_r'
collect2.exe: error: ld returned 1 exit status

@capocasa
Copy link
Copy Markdown
Collaborator Author

Testing platforms

@capocasa capocasa force-pushed the devel2 branch 15 times, most recently from a52d050 to abd8ace Compare March 17, 2026 09:46
@capocasa
Copy link
Copy Markdown
Collaborator Author

capocasa commented Mar 17, 2026

Done- replaces non-thread-safe calls that glibc and windows keep around for their extreme backwards compatibility with respective thread-safe extensions, does not touch anything else.

There's one hitch, I resorted to an ugly hack to prevent a musl regression. Clean way forward has been previously discussed.

@capocasa capocasa marked this pull request as ready for review March 17, 2026 13:28
let protoent = winlean.getprotobyname(name.cstring)
elif defined(linux) and not defined(android):
var protoent: ptr posix.Protoent
{.emit: ";\n#ifdef __GLIBC__".}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's too ugly, we need to find a better solution for it. Or we simply add our own locks around the existing getprotobyname() calls. The new Muslim libC is the future; if it lacks the new awesome getprotobyname_r with its 5 parameter ptr ptr shit we can't use it.

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

Successfully merging this pull request may close these issues.

2 participants