2017883 - PR_CreateThread on Windows doesn't call PR_SetError on _beginthreadex failure

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Jens Stutte [:jstutte]]

Unfortunately it seems that we cannot call GetLastError after PR_CreateThread as the cleanup on the error path seems to overwrite it with ERROR_SUCCESS, so there seems to be no viable workaround. I came here through bug 2015433 while trying to improve diagnostics.

Following Claude's detailed findings:

The "Last Error" on all Windows crash reports for this signature is ERROR_SUCCESS. I investigated what internal code condition could produce this and found that NSPR's cleanup path after _beginthreadex failure calls TlsGetValue, which masks GetLastError().

Background: Firefox uses the WIN95 NSPR target on Windows

Per moz.build, Firefox defines WIN95=True for NSPR, so the thread implementation used is w95thred.c, not ntthread.c. _PR_USE_STATIC_TLS is not defined, so _MD_GET_ATTACHED_THREAD() unconditionally expands to TlsGetValue(_pr_currentThreadIndex) at compile time.

TlsGetValue is documented by Microsoft to call SetLastError(ERROR_SUCCESS) on success.

The masking sequence

When _beginthreadex fails in _PR_MD_CREATE_THREAD, the function simply returns PR_FAILURE without capturing GetLastError() and without calling PR_SetError(). At this point GetLastError() still holds the real Windows error from _beginthreadex/CreateThread.

Control returns to _PR_NativeCreateThread, which runs cleanup code that calls _PR_DecrActiveThreadCount. That function calls PR_Lock(_pr_activeLock), which immediately does:

PRThread* me = _PR_MD_CURRENT_THREAD();

There might be value in checking other functions if they lack proper error propagation of GetLastError on Windows.

Comment 1

[Jens Stutte [:jstutte]]

Attachment: Bug 2017883 - Call PR_SetError on failure in w95thred.c thread functions. r?kaie

Comment 2

[Jens Stutte [:jstutte]]

Attachment: Bug 2017883 - Call PR_SetError on failure in w95thred.c thread functions. r?kaie

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:KaiE, could you have a look please?

For more information, please visit BugBot documentation.

Comment 4

[Kai Engert [:KaiE:]]

https://hg-edge.mozilla.org/projects/nspr/rev/be49e29ddbd27c570e95c86a4ad12d4d70ba30bc