1313351 - Intermittent leakcheck | default process: 8 bytes leaked (WasmModule)

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Treeherder Bug Filer]

Filed by: tomcat [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=38290574&repo=mozilla-inbound

https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-linux-debug/1477550613/mozilla-inbound_ubuntu32_vm-debug_test-mochitest-devtools-chrome-2-bm141-tests1-linux32-build38.txt.gz

Comment 1

[Intermittent Failures Robot]

20 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 10
* autoland: 6
* fx-team: 3
* mozilla-central: 1

Platform breakdown:
* linux32: 15
* linux64: 5

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1313351&startday=2016-10-24&endday=2016-10-30&tree=all

Comment 2

[Intermittent Failures Robot]

37 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 16
* autoland: 14
* mozilla-central: 4
* try: 3

Platform breakdown:
* linux32: 23
* linux64: 13
* android-4-3-armv7-api15: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1313351&startday=2016-10-31&endday=2016-11-06&tree=all

Comment 3

[Luke Wagner [:luke]]

Ok, so reading AtomicRefCounted.h, it only does the leak-checking if MOZILLA_INTERNAL_API is #defined and this does not seem to be the case in WasmJS.cpp (where Release() is called which is likely being inlined) so that is definitely a problem.  I don't think this is as simple as #defining MOZILLA_INTERNAL_API within SM either since that #includes nsXPCOM.h.  So I'm thinking we need to turn js::RefCounted into a stripped clone of mozilla::RefCounted instead of doing the 'using'.

Comment 4

[Luke Wagner [:luke]]

Attachment: fix-ref-counted

This patch just clones mozilla::(Atomic)RefCounted into js/public/RefCounted.h, stripping out the leak-checking goo.  It's important that the clone implement the same interface so that it works for mozilla::RefPtr which we reuse as well.  With the leak-checking goo gone, very little code remains so, for readability, I untemplated the impl.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8821590 [details] [diff] [review]
fix-ref-counted

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

Thanks!

::: js/public/RefCounted.h
@@ +29,5 @@
>  
> +  protected:
> +    RefCounted() : mRefCnt(0) {}
> +    ~RefCounted() { MOZ_ASSERT(mRefCnt == DEAD); }
> +  

Nit: trailing whitespace here and a few times below.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/47d317cfa267
Fix js::RefCounted to not do leak checking. r=jandem

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/39a2a96f6cda
followup - Also back out the ElfLoader changes from bug 1320905 on a CLOSED TREE. r=red

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/39a2a96f6cda

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/47d317cfa267