1301796 - Fix potential race in RegisterJSRuntime

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

This method uses a static bool to detect if the CC has been registered as a memory reporter yet. We now have CC on workers in addition to the main thread, so potentially multiple threads are accessing this single variable. I think it isn't a problem in practice because the main thread's CC runtime should be completely started up before we start any worker CC runtime, but it still feels questionable. I also am not sure why this boolean is needed, as RegisterJSRuntime should only be called once per CC.

Comment 1

[Andrew McCreight [:mccr8]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e2410768ddf

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1301796 - Remove racy code in nsCycleCollector::RegisterJSRuntime.

Review commit: https://reviewboard.mozilla.org/r/78446/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/78446/

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8790780 [details]
Bug 1301796 - Remove racy code in nsCycleCollector::RegisterJSRuntime.

https://reviewboard.mozilla.org/r/78446/#review76968

And we don't want worker CCRuntimes to be registered or something?
(but that would be a different bug)

Comment 4

[Andrew McCreight [:mccr8]]

(In reply to Olli Pettay [:smaug] from comment #3)
> And we don't want worker CCRuntimes to be registered or something?
> (but that would be a different bug)

I'm not entirely sure how memory reporter registration workers for non-mainthread. Possibly it just works. Anyways, I already have bug 1283251 on file.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0f7f9fa31c4a
Remove racy code in nsCycleCollector::RegisterJSRuntime. r=smaug

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0f7f9fa31c4a