(In reply to Olli Pettay [:smaug] from comment #10) > CC runs in the same thread as cycle collectable objects, so there can't be really a race there. > If someone doesn't keep the object alive while construction process is ongoing, and let's JS to be called, the object may get deleted. > That isn't really a CC thing, but normal refcounting. (CC may just postpone deletion when refcnt drops to 0, but if event loop spins or so, there may not be delay. If object isn't CCable, refcnt dropping to 0 means instant deletion) > > https://searchfox.org/mozilla-central/rev/f98dad153b59a985efd4505912588d4651033395/dom/serviceworkers/ServiceWorkerRegistrationChild.cpp#29 > Nothing keeps mOwner alive if UpdateState triggers JS which may kill ServiceWorkerRegistrationChild. > > https://searchfox.org/mozilla-central/rev/f98dad153b59a985efd4505912588d4651033395/dom/serviceworkers/RemoteServiceWorkerRegistrationImpl.cpp#153 > Nothing keeps mOwner alive here either if UpdateState triggers JS which may kill RemoteServiceWorkerRegistrationImpl > And based on the stack, some DOM event is dispatched underneath, and DOM Events may have JS implemented listeners which get then called > synchronously. So I take away from this that it is a bad idea to push events during the construction phase of a CCP. And that if we do not do so, everything should be fine, as then the CC can't chip in. It is something to keep in mind and easily violated, though. > > This isn't about CC, but seems to about normal COM-rules. Caller should keep the callee alive. > Obvious I was just looking around the code, didn't debug this. So could be wrong. But caller not keeping callee alive is a rather common mistake in refcounted systems. > Annotating the code with MOZ_CAN_RUN_SCRIPT might be useful. > https://searchfox.org/mozilla-central/rev/f98dad153b59a985efd4505912588d4651033395/mfbt/Attributes.h#503-511 I assume, this does not change the behavior but enforces us to keep references in the callee (and helps us to keep in mind the above rule). And that it is not meant to be applied to a constructor. Thanks!
Bug 1607276 Comment 11 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
(In reply to Olli Pettay [:smaug] from comment #10) > CC runs in the same thread as cycle collectable objects, so there can't be really a race there. > If someone doesn't keep the object alive while construction process is ongoing, and let's JS to be called, the object may get deleted. > That isn't really a CC thing, but normal refcounting. (CC may just postpone deletion when refcnt drops to 0, but if event loop spins or so, there may not be delay. If object isn't CCable, refcnt dropping to 0 means instant deletion) > > https://searchfox.org/mozilla-central/rev/f98dad153b59a985efd4505912588d4651033395/dom/serviceworkers/ServiceWorkerRegistrationChild.cpp#29 > Nothing keeps mOwner alive if UpdateState triggers JS which may kill ServiceWorkerRegistrationChild. > > https://searchfox.org/mozilla-central/rev/f98dad153b59a985efd4505912588d4651033395/dom/serviceworkers/RemoteServiceWorkerRegistrationImpl.cpp#153 > Nothing keeps mOwner alive here either if UpdateState triggers JS which may kill RemoteServiceWorkerRegistrationImpl > And based on the stack, some DOM event is dispatched underneath, and DOM Events may have JS implemented listeners which get then called > synchronously. So I take away from this that it is a bad idea to push JS events during the construction phase of a CCP. And that if we do not do so, everything should be fine, as then the CC can't chip in. It is something to keep in mind and easily violated, though. > > This isn't about CC, but seems to about normal COM-rules. Caller should keep the callee alive. > Obvious I was just looking around the code, didn't debug this. So could be wrong. But caller not keeping callee alive is a rather common mistake in refcounted systems. > Annotating the code with MOZ_CAN_RUN_SCRIPT might be useful. > https://searchfox.org/mozilla-central/rev/f98dad153b59a985efd4505912588d4651033395/mfbt/Attributes.h#503-511 I assume, this does not change the behavior but enforces/reminds us to keep references in the caller (and helps us to keep in mind the above rule). And that it is not meant to be applied to a constructor. Thanks!