1380923 - Intermittent test leakage of XUL document from NotifyOffThreadScriptCompletedRunnable::sReceivers

Status: NEW

(Core :: XUL, enhancement, P3)

Description

[Ting-Yu Chou [:ting] (away)]

Copy from bug 1365133 comment 195:

<copy>
From cc-edges.1093.test.1499754765787.log, the root of leaked window is a XUL document with 1 unknown edge, for instance:

  --DOMWINDOW == 3 (0x7f26ffcfa800) [pid = 1093] [serial = 44] [outer = (nil)] [url = about:preferences#privacy]

  ting@ting-pc:~/w/fx/mc$ python ../tools/heapgraph/find_roots.py ~/Desktop/cc-edges.1093.test.1499754765787.log 0x7f26ffcfa800
  Parsing /home/ting/Desktop/cc-edges.1093.test.1499754765787.log. Done loading graph. 

  0x7f26f7af9000 [nsDocument normal (XUL) about:preferences#privacy]
      --[mChildren[i]]--> 0x7f27159ff160 [FragmentOrElement (XUL) page about:preferences#privacy]
      --[Preserved wrapper]--> 0x7f2705995040 [JS Object (XULElement)]
      --[group_global]--> 0x7f27051f91a0 [JS Object (Window)]
      --[UnwrapDOMObject(obj)]--> 0x7f26f68b5000 [nsGlobalWindow # 46 inner about:preferences#privacy]
      --[mOuterWindow]--> 0x7f26ffcfa800 [nsGlobalWindow # 44 outer ]

      Root 0x7f26f7af9000 is a ref counted object with 1 unknown edge(s).
      known edges:
         0x7f26f68b5000 [nsGlobalWindow # 46 inner about:preferences#privacy] --[mDoc]--> 0x7f26f7af9000
         0x7f26f710f9b0 [nsNodeInfoManager] --[mDocument]--> 0x7f26f7af9000
         0x7f26f60c4790 [nsXULCommandDispatcher] --[mDocument]--> 0x7f26f7af9000
         0x7f2705995190 [JS Object (XULDocument)] --[UnwrapDOMObject(obj)]--> 0x7f26f7af9000

And I noticed this special warning from nsFrameLoader::LoadURI after "TEST-START | Shutdown" for "about:blank", which happens when insert the <browser> [1] after the sync.js [2] gets compiled off main thread:

  WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file /home/worker/workspace/build/src/dom/base/nsFrameLoader.cpp, line 328

This leads me to NotifyOffThreadScriptCompletedRunnable and its sReceivers [3], which seems is where the unknown edge from. My theory is the test loads preferences.xul but finishes before the last NotifyOffThreadScriptCompletedRunnable::Run() to release [4] the reference to the XUL document. I did an experiment by adding a sleep in the beginning of OffThreadScriptReceiverCallback() so the NotifyOffThreadScriptCompletedRunnable::Run() is delayed, then I can reproduce leaked windows of "about:preferences#privacy" reliably by running the test:

  ./mach mochitest --disable-e10s ./browser/base/content/test/alerts/browser_notification_open_settings.js

:mccr8, do we really need to keep the XUL document alive just for passing it the compiled script? If we don't, shouldn't we use WeakPtr for storing the mReceiver instead and get rid of sReceiver? Please let me know if I should ask someone else for this.
</copy>

Comment 1

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #0)
> :mccr8, do we really need to keep the XUL document alive just for passing it
> the compiled script? If we don't, shouldn't we use WeakPtr for storing the
> mReceiver instead and get rid of sReceiver? Please let me know if I should
> ask someone else for this.

:smaug, I've checked bug 1155328 and seems we don't need to keep the XUL document alive to pass it the compiled script, is my understanding correct?

Comment 2

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

Why we aren't releasing the strong reference?


But using nsWeakPtr is probably ok.

Comment 3

[Ting-Yu Chou [:ting] (away)]

Attachment: wip

Comment 4

[Ting-Yu Chou [:ting] (away)]

With the WIP, I am still seeing leaked windows when there's a sleep() in OffThreadScriptReceiverCallback(). I'll check this further.