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

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Steve Fink [:sfink] [:s:]]

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.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: 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.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8334907 [details] [diff] [review]
Move XMLHttpRequest::StateData off of the stack to silence analysis

hup!

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

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.

Comment 4

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/972924aa7110

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/972924aa7110