940781 - [email/backend] GELAM Test runner can hang if a garbage collection eats the progress listener's weak reference

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

:mcav and I were experiencing really consistent test hangs today.  I tracked down the problem, and it was that the nsIWebProgressListener was getting garbage collected because it uses a weak reference.  Although I was clever enough to create a global strong reference, I was not clever enough to avoid zeroing it out almost immediately.

There is some asynchrony going on; probably some system change impacted the sequencing.

Fix up momentarily.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Attachment: diff -w of the fix with lots of extra debugging added

(Note I used -w because I did some minor re-indenting, so there are some indentations that will look weird in the patch but they are totally not weird in the actual fix.)

What the patch does in rough order of reading the patch:

- Decode flags in progress listener output and list the request URL in use.

- Makes the progress listener ignore requests involving about:blank.  This lets us add the progress listener before we set the 'src' attribute on the iframe, allowing us to see the entire start/stop cycle for the iframe.  Previously we set the src before creating the iframe to avoid getting faked out by this.

- Adds harness logging for garbage collections (JS in-compartment garbage collections) and cycle collections (gecko-wide cycle collection across all JS compartments and through XPCOM.)  This helped make it obvious that our weak reference was getting eaten.

- Removes some redundant global poking in favor of our now much more reliable progress listening strategy.

- Stops clearing gProgress.  This was the root problem, I believe.

- The ordering alluded to above so we have the listener listening to the entire life-cycle of our load.

Comment 2

[Marcus Cavanaugh [:mcav]]

Comment on attachment 8334979 [details] [diff] [review]
diff -w of the fix with lots of extra debugging added

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

What an ugly bug. LGTM.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

landed in GELAM/master on a green travis:
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/261
https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/26709308504ae24feea4345f5aa2a6e39e674917

(I realize too late this would have been an awesome time to use Shepherd.)