Move XMLHttpRequest::StateData off of the stack to silence analysis

RESOLVED FIXED in mozilla28

Status

()

Core
DOM
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: sfink, Assigned: sfink)

Tracking

unspecified
mozilla28
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Hazard:

Function 'XMLHttpRequest.cpp:uint8 {anonymous}::EventRunnable::WorkerRun(JSContext*, mozilla::dom::workers::WorkerPrivate*)' has unrooted 'state' of type 'mozilla::dom::workers::XMLHttpRequest::StateData' live across GC call 'uint8 JSAutoStructuredCloneBuffer::read(JSContext*, class JS::MutableHandle<JS::Value>, JSStructuredCloneCallbacks*, void*)' at dom/workers/XMLHttpRequest.cpp:715

This hazard is really a weakness of the analysis, because there's a StateDataAutoRooter in scope. But StateData contains a Heap<Value>, and it's a little funky to put one of those on the stack. Funky, but allowed as long as you trace it, and the StateDataAutoRooter will achieve that.
(Assignee)

Comment 1

4 years ago
Created attachment 8334907 [details] [diff] [review]
Move XMLHttpRequest::StateData off of the stack to silence analysis

Still, it's simpler to always have StateData on the heap, especially since that will silence the analysis hazard as a side effect. We still need the AutoRooter to trace it. Once MOZ_HEAP_CLASS is implemented, we'll be able to mark StateData with it.
Attachment #8334907 - Flags: review?(jonas)
Comment on attachment 8334907 [details] [diff] [review]
Move XMLHttpRequest::StateData off of the stack to silence analysis

hup!
Attachment #8334907 - Flags: review?(jonas) → review?(bent.mozilla)
Comment on attachment 8334907 [details] [diff] [review]
Move XMLHttpRequest::StateData off of the stack to silence analysis

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

::: dom/workers/XMLHttpRequest.cpp
@@ +681,5 @@
>          mProxy->mLastTotal = mTotal;
>        }
>      }
>  
> +    ScopedDeletePtr<XMLHttpRequest::StateData> state(new XMLHttpRequest::StateData());

Nit: Please use nsAutoPtr.
Attachment #8334907 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 4

4 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/972924aa7110
https://hg.mozilla.org/mozilla-central/rev/972924aa7110
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28

Updated

4 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.