Skip to content

curl_threads: always use native threads/mutex on Windows - #22593

Closed
vszakats wants to merge 2 commits into
curl:masterfrom
vszakats:lib-win-native-threads
Closed

vszakats wants to merge 2 commits into
curl:masterfrom
vszakats:lib-win-native-threads

Conversation

@vszakats

@vszakats vszakats commented Aug 15, 2026

Copy link
Copy Markdown
Member

Syncing with build systems, which already enforce this.


easy_lock.h also had this precedence before this patch.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Prioritizes native Windows threading over POSIX threads in curl_threads.c.

Changes:

  • Reorders thread implementation selection.
  • Reorders mutex and condition-variable implementation selection.
Suppressed comments (1)

lib/curl_threads.c:139

  • The mutex/condition implementation has also switched to Win32 precedence, while curl_threads.h:29-56 still selects pthread_mutex_t, pthread_cond_t, and pthread_t whenever HAVE_THREADS_POSIX is defined. With both macros set, these definitions conflict with the public declarations and Win32 callers here. Reorder the header conditional consistently with this branch.
#ifdef _WIN32

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/curl_threads.c
Syncing with build systems, which already did so, skipping pthreads
detection on Windows.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@vszakats vszakats added the Windows Windows-specific label Aug 15, 2026
@vszakats vszakats changed the title curl_threads: always use native threads on Windows curl_threads: always use native threads/mutex on Windows Aug 15, 2026
@MichalPetryka

Copy link
Copy Markdown
Contributor

Does it make sense to conditionally use the faster Win8+ WaitOnAddress/WakeByAddress*?

@jay

jay commented Aug 15, 2026

Copy link
Copy Markdown
Member

Does it make sense to conditionally use the faster Win8+ WaitOnAddress/WakeByAddress*?

I've never seen that used in any project that I've worked on. How do you know it is faster? Also, it has some caveats:

Note: WaitOnAddress is guaranteed to return when the address is signaled, but it is also allowed to return for other reasons. For this reason, after WaitOnAddress returns the caller should compare the new value with the original undesired value to confirm that the value has actually changed. For example, the following circumstances can result in waking the thread early:

  • Low memory conditions
  • A previous wake on the same address was abandoned
  • Executing code on a checked build of the operating system

@vszakats

Copy link
Copy Markdown
Member Author

Does it make sense to conditionally use the faster Win8+ WaitOnAddress/WakeByAddress*?

In general, I'm not a big fan of doing version-conditional things (esp. runtime-conditional)
unless the benefit is a meaningful one for most users. Were the perf benefits measured
by someone? These APIs seem more than just replacements, so it's not trivial to adapt the
code to test?

Also seconding Jay, that new/rarely used features may have suprises or bugs. For
locks/threading it may be nasty.

vszakats added a commit to vszakats/curl that referenced this pull request Aug 16, 2026
To remove a build difference between mingw-w64 and MSVC builds, simplify
and save the cost of detection/verification in CI.

`gettimeofday()` was used in `curl_threads.c` in codepath practically
unreachable in Windows builds, and in `tool_writeout.c` that was a
fallback to `time()` for targets without it.

Ref: curl#22593
@vszakats vszakats closed this in 5b9946a Aug 16, 2026
@vszakats
vszakats deleted the lib-win-native-threads branch August 16, 2026 10:17
vszakats added a commit to vszakats/curl that referenced this pull request Aug 16, 2026
To remove a build difference between mingw-w64 and MSVC builds, simplify
and save the cost of detection/verification in CI.

`gettimeofday()` was used in `curl_threads.c` in codepath practically
unreachable in Windows builds, and in `tool_writeout.c` that was a
fallback to `time()` for targets without it.

Ref: curl#22593
vszakats added a commit that referenced this pull request Aug 16, 2026
To remove a build difference between mingw-w64 and MSVC builds (where
mingw-w64 has it, while MSVC does not). Also to simplify and save the
cost of detection/verification in CI.

`gettimeofday()` was used in `curl_threads.c` in codepath practically
unreachable in Windows builds, and in `tool_writeout.c` that falls back
to `time()` for targets without it.

Ref: #22593

Closes #22594
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tidy-up Windows Windows-specific

Development

Successfully merging this pull request may close these issues.

4 participants