1168169 - Change nsThreadShutdownContext::joiningThread to use nsRefPtr and mark as MOZ_STACK_CLASS

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nika Layzell [:nika] (ni? for response)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=97d050105ff2

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Change nsThreadShutdownContext::joiningThread to use nsRefPtr and mark as MOZ_STACK_CLASS

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8610197 [details] [diff] [review]
Change nsThreadShutdownContext::joiningThread to use nsRefPtr and mark as MOZ_STACK_CLASS

Review of attachment 8610197 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the change below.

::: xpcom/threads/nsThread.cpp
@@ +235,2 @@
>  {
> +  nsRefPtr<nsThread> joiningThread;

I don't think there's any need for refcounting this; the thread manager is holding a reference to |joiningThread| and isn't going to let that go while we're running the code in question.  Like other cases, this can be MOZ_NON_OWNING_REF.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Attachment: Mark nsThreadShutdownContext::joiningThread as MOZ_UNSAFE_REF and mark as MOZ_STACK_CLASS

I think that MOZ_UNSAFE_REF would be a more appropriate annotation in this scenario.

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8623092 [details] [diff] [review]
Mark nsThreadShutdownContext::joiningThread as MOZ_UNSAFE_REF and mark as MOZ_STACK_CLASS

Review of attachment 8623092 [details] [diff] [review]:
-----------------------------------------------------------------

*shrug*  OK.

Comment 5

[Nathan Froyd [:froydnj]]

You want to land this? :)

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Attachment: Mark nsThreadShutdownContext::joiningThread as MOZ_UNSAFE_REF and mark as MOZ_STACK_CLASS

I guess this is what happens when you forget to mark yourself as the assignee :P.

MozReview-Commit-ID: 16ebijitZcz

Comment 7

[Nika Layzell [:nika] (ni? for response)]

Attachment: Mark nsThreadShutdownContext::joiningThread as MOZ_UNSAFE_REF

Looking at this more it seems like the situation has changed since this patch
was originally written. nsThreadShutdownContext is allocated on the heap behind
a nsAutoPtr, so we can't mark it as MOZ_STACK_CLASS.

MozReview-Commit-ID: 16ebijitZcz

Comment 8

[Nathan Froyd [:froydnj]]

Comment on attachment 8858896 [details] [diff] [review]
Mark nsThreadShutdownContext::joiningThread as MOZ_UNSAFE_REF

Review of attachment 8858896 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.

Does the static analysis look through templated types to find raw pointers?  The documentation for MOZ_UNSAFE_REF says it only applies to pointers...please be sure that you're not going to break the static analysis jobs before you land this.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/14f4b3a2ca15
Mark nsThreadShutdownContext::joiningThread as MOZ_UNSAFE_REF, r=froydnj

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/14f4b3a2ca15