git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[RFC] Thread safety in some low-level functions

From
Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Date
Jun 24, 2020, 22:29 UTC
Message-ID
<CAHd-oW5zh=BG29O0Z-M7R26Lgd=RHECMV2+qByF+vU6PmrEn_Q@mail.gmail.com>
Hi,

I'm working with threads in unpack-trees and noticed that [warning|error]_errno() uses strerror(), which is not thread-safe. We could try to avoid calling these functions in threaded code, but they are sometimes too deep in the call stack to be noticed... (or even avoided). The same happens with oid_to_hex(), which writes to a static buffer.

I don't think I've ever seen a bug report involving these functions being called racily, but this possibility is not currently excluded in our codebase. For example, see grep_source_load_file(), which is called by multiple threads concurrently and might call the thread-unsafe error_errno(). (Although, I admit, the chance of a race here must be very low...)

I still haven't been able to come up with a simple / easy change that could make these functions thread safe, but here are my thoughts so far:

- For strerror(), there is a thread-safe variant: strerror_r().
However IIUC, this variant is not present on Windows (although there
is strerror_s() which *seems* to be somewhat similar). Also, there are
two versions of strerror_r() on Linux: one is XSI-compliant and the
other is GNU-specific. I don't know what the situation is in other
OSes...
- Regarding, oid_to_hex(), a patch from 2010 [1] proposed a solution
using thread-local storage and pthread_once(). But as Hannes pointed
out in this other thread [2] , implementing a Windows equivalence for
pthread_once() could be tricky and voluminous. Since this thread dates
from 7 years ago, I was wondering if we would be able to implement it
nowadays with InitOnceExecuteOnce() [3].

Finally, leaving these functions thread-unsafe is also a possibility... As I mentioned earlier, they don't seem to be causing problems out there for now (at least not reported). But if we can find a feasible solution to introduce thread-safety, I think it would be great. The codebase would be more robust and we would be able to work with threads with much more confidence.

Any thoughts?

Thanks, Matheus

[1] https://lore.kernel.org/git/20100323173130.GC4218@fredrik-laptop/ [2] https://lore.kernel.org/git/516D5CA4.7000500@viscovery.net/ [3]: https://docs.microsoft.com/en-us/windows/win32/sync/using-one-time-initialization

Next: Matheus Tavares Bernardino
Message 1 of 25 in “[RFC] Thread safety in some low-level functions”
  1. Matheus Tavares BernardinoJun 24, 2020
  2. Matheus Tavares BernardinoJun 24, 2020
  3. brian m. carlsonJun 25, 2020
  4. 0/2 Make oid_to_hex() thread-safeMatheus Tavares, Jun 25, 2020
  5. 1/2 compat/win32/pthread: add pthread_once()Matheus Tavares, Jun 25, 2020
  6. Christian CouderJun 26, 2020
  7. Matheus Tavares BernardinoJun 26, 2020
  8. 2/2 hex: make hash_to_hex_algop() and friends thread-safeMatheus Tavares, Jun 25, 2020
  9. brian m. carlsonJun 25, 2020
  10. Matheus Tavares BernardinoJun 25, 2020
  11. Matheus Tavares BernardinoJun 26, 2020
  12. Christian CouderJun 26, 2020
  13. Christian CouderJun 26, 2020
  14. Matheus Tavares BernardinoJun 26, 2020
  15. 0/2 Make oid_to_hex() thread-safeMatheus Tavares, Jun 26, 2020
  16. 1/2 compat/win32/pthread: add pthread_once()Matheus Tavares, Jun 26, 2020
  17. 2/2 hex: make hash_to_hex_algop() and friends thread-safeMatheus Tavares, Jun 26, 2020
  18. Johannes SchindelinJun 29, 2020
  19. Matheus Tavares BernardinoJun 30, 2020
  20. Johannes SchindelinJul 1, 2020
  21. Johannes SchindelinJul 16, 2020
  22. Matheus Tavares BernardinoJul 18, 2020
  23. Johannes SchindelinAug 10, 2020
  24. Matheus TavaresJul 18, 2020
  25. René ScharfeJul 26, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.