256615 - crash when calling nsIThread.state before calling nsIThread.init [@ PR_GetThreadState]

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[timeless]

steps:
run xpcshell
execute:
Components.classes["@mozilla.org/thread;1"].createInstance(Components.interfaces.nsIThread).state;

actual results:
 	nspr4.dll!PR_GetThreadState(const PRThread * thread=0x00000000)  Line 445 + 0x4	C
>	xpcom.dll!nsThread::GetState(PRThreadState * result=0x0012e7c0)  Line 232 +
0x9	C++
 	xpcom.dll!XPTC_InvokeByIndex(nsISupports * that=0x0012e944, unsigned int
methodIndex=0x00fb219c, unsigned int paramCount=0x0185b008, nsXPTCVariant *
params=0x00000008)  Line 102	C++

expected results: exception (NS_ERROR_NOT_INITIALIZED)

Comment 1

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: Patch v1

Comment 2

[Doug Turner (:dougt)]

hmm, lets either protect all of the methods that access mThread or add comments
to the idl that suggest calling into any other method without a successful call
to init is undefined.

Comment 3

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

<CTho> dougt: so we should add an assert then, too, since those functions
shoudln't be called, right? 
<dougt> that would be nice too.  let developers (using debug builds) know that
there is a problem.

I'll go through and edit all of the functions that currently break if mThread is
null.

Comment 4

[timeless]

ctho: fwiw a PR_ASSERT is fatal, it will intentionally terminate the app if it fails.

Comment 5

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

So do I add a null check and no assert and early return, a null check and a
non-fatal assert and early return, or a fatal assert?

Comment 6

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch v2

Null check, early return, no asserts.

Comment 7

[Doug Turner (:dougt)]

Comment on attachment 157019 [details] [diff] [review]
patch v2

looks great.  thanks for the fix.

Comment 8

[timeless]

mozilla/xpcom/threads/nsThread.cpp 	1.56