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

RESOLVED FIXED in Firefox 53

Status

()

Core
General
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: Treeherder Bug Filer, Assigned: luke)

Tracking

({intermittent-failure, memory-leak})

unspecified
mozilla53
intermittent-failure, memory-leak
Points:
---

Firefox Tracking Flags

(firefox51 unaffected, firefox52 unaffected, firefox53 fixed)

Details

Attachments

(1 attachment)

Updated

2 years ago
Keywords: mlk

Comment 1

2 years ago
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

2 years ago
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
(Assignee)

Comment 3

a year ago
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'.
(Assignee)

Comment 4

a year ago
Created attachment 8821590 [details] [diff] [review]
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.
Assignee: nobody → luke
Status: NEW → ASSIGNED
Attachment #8821590 - Flags: review?(jdemooij)
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.
Attachment #8821590 - Flags: review?(jdemooij) → review+

Comment 6

a year ago
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/47d317cfa267
Fix js::RefCounted to not do leak checking. r=jandem

Comment 7

a year ago
Pushed by jandemooij@gmail.com:
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

a year ago
backoutbugherder
https://hg.mozilla.org/mozilla-central/rev/39a2a96f6cda
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
status-firefox51: --- → unaffected
status-firefox52: --- → unaffected
You need to log in before you can comment on or make changes to this bug.