Assorted Win32 error handling fixes (CreateThread, pgwin32_select, pg_usleep)

Started by Sehrope Sarkuni3 months ago2 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliessuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t139924
psql -h localhost -U postgres

Built from patchset v2 (message #2), August 29, 2026 at 08:03 PM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t139924_2 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t139924_2 && git checkout t139924_2

Patchset v2 (message #2) is on t139924_2

Jump to latest
#1Sehrope Sarkuni
sehrope@jackdb.com

Hi-

While trying out some new analysis tooling, I noticed a few small bugs
in error handling. Attached are four patches:

0001: CreateThread() returns NULL on failure[1]https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createthread#return-value, not
INVALID_HANDLE_VALUE, so the failure check in the win32 timer code
could never fire. A failed thread creation would silently disable all
timer-based timeouts rather than reporting FATAL.

0002: Same CreateThread() return value mixup in pg_test_fsync.

0003: The events array in pgwin32_select() is sized for 2*FD_SETSIZE
socket events, but the signal event is appended after the sockets. If
the read and write fd sets are completely disjoint and full, the
signal event is written one element past the end of the array. Fixed
by sizing the array as 2*FD_SETSIZE + 1.

0004: pg_usleep() on win32 only checks for WAIT_OBJECT_0 from
WaitForSingleObject()[2]https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitforsingleobject#return-value, so WAIT_FAILED is silently treated the same as
a timeout, i.e. an immediate return. A caller using pg_usleep() in a
retry loop would spin at full speed if the event handle were ever
bad. Changed to handle WAIT_OBJECT_0 and WAIT_TIMEOUT explicitly and
elog(FATAL) on anything else.

0004 deserves some eyeballs since it changes behavior on the
failure path rather than just fixing a dead check. The first three
are mechanical.

Passes all checks with the new GitHub Actions CI:
https://github.com/sehrope/postgres/actions/runs/27282715049

[1]: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createthread#return-value
https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createthread#return-value
[2]: https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitforsingleobject#return-value
https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitforsingleobject#return-value

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/

Attachments:

t139924_1
0001-Fix-CreateThread-failure-check-in-win32-timer-code.patchapplication/octet-stream; name=0001-Fix-CreateThread-failure-check-in-win32-timer-code.patchDownload+1-2
0002-Fix-CreateThread-failure-check-in-pg_test_fsync.patchapplication/octet-stream; name=0002-Fix-CreateThread-failure-check-in-pg_test_fsync.patchDownload+1-3
0003-Fix-off-by-one-in-pgwin32_select-event-array.patchapplication/octet-stream; name=0003-Fix-off-by-one-in-pgwin32_select-event-array.patchDownload+4-4
0004-Do-not-treat-a-failed-wait-as-a-timeout-in-win32-pg_.patchapplication/octet-stream; name=0004-Do-not-treat-a-failed-wait-as-a-timeout-in-win32-pg_.patchDownload+11-7
#2Sehrope Sarkuni
sehrope@jackdb.com
In reply to: Sehrope Sarkuni (#1)
Re: Assorted Win32 error handling fixes (CreateThread, pgwin32_select, pg_usleep)

Attached is v2, rebased over master. Only 0003 changed to remove a
declaration it had as context. The fixes are identical to v1.

Of the four, only 0001 has any practical server impact. If CreateThread
fails, every timer-based timeout in that process is silently disabled
for the rest of its lifetime. 0002 can hang pg_test_fsync. 0003 and
0004 are defensive fixes with no reachable in-core trigger.

I'll add this to the next commitfest.

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/

Attachments:

t139924_2
v2-0003-Fix-off-by-one-in-pgwin32_select-event-array.patchapplication/x-patch; name=v2-0003-Fix-off-by-one-in-pgwin32_select-event-array.patchDownload+4-4
v2-0004-Do-not-treat-a-failed-wait-as-a-timeout-in-win32-.patchapplication/x-patch; name=v2-0004-Do-not-treat-a-failed-wait-as-a-timeout-in-win32-.patchDownload+11-7
v2-0001-Fix-CreateThread-failure-check-in-win32-timer-cod.patchapplication/x-patch; name=v2-0001-Fix-CreateThread-failure-check-in-win32-timer-cod.patchDownload+1-2
v2-0002-Fix-CreateThread-failure-check-in-pg_test_fsync.patchapplication/x-patch; name=v2-0002-Fix-CreateThread-failure-check-in-pg_test_fsync.patchDownload+1-3