Closed Bug 1524478 Opened 2 years ago Closed 1 year ago

Intermittent LeakSanitizer | leak at std::sys::unix::alloc::_$LT$impl$u20$core..alloc..GlobalAlloc$u20$for$u20$std..alloc..System$GT$::alloc, __rdl_alloc, alloc::alloc::alloc, _$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Alloc$GT$::alloc

Categories

(Core :: DOM: Web Authentication, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox-esr60 --- fixed
firefox66 --- wontfix
firefox67 --- fixed
firefox68 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: keeler)

References

Details

(Keywords: intermittent-failure, memory-leak, Whiteboard: [stockwell disable-recommended])

Attachments

(2 files)

#[markdown(off)]
Filed by: aciure [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=225371155&repo=autoland

https://queue.taskcluster.net/v1/task/KIz3cXzpT_mDkBseK0_U8g/runs/0/artifacts/public/logs/live_backing.log

[task 2019-02-01T05:37:59.001Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at AllocateNewAtom, atomizeAndCopyChars, AtomizeAndCopyCharsFromLookup, AtomizeAndCopyChars
[task 2019-02-01T05:37:59.001Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame AllocateNewAtom matched a expected leak
[task 2019-02-01T05:37:59.002Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at js::frontend::BytecodeEmitter::emitTree, emitStatementList, js::frontend::BytecodeEmitter::emitTree, emitLexicalScopeBody
[task 2019-02-01T05:37:59.002Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame js::frontend::BytecodeEmitter::emitTree matched a expected leak
[task 2019-02-01T05:37:59.002Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at js::RunScript, js::ExecuteKernel, ExecuteInExtensibleLexicalEnvironment, Constructor
[task 2019-02-01T05:37:59.003Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame js::RunScript matched a expected leak
[task 2019-02-01T05:37:59.003Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at js::ExecuteInJSMEnvironment, mozJSComponentLoader::ObjectForLocation, js_arena_malloc, js_pod_arena_malloc
[task 2019-02-01T05:37:59.003Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame js::ExecuteInJSMEnvironment matched a expected leak
[task 2019-02-01T05:37:59.004Z] 05:37:59 INFO - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at std::sys::unix::alloc::_$LT$impl$u20$core..alloc..GlobalAlloc$u20$for$u20$std..alloc..System$GT$::alloc, __rdl_alloc, alloc::alloc::alloc, _$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Alloc$GT$::alloc
[task 2019-02-01T05:37:59.004Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at js::StringBuffer::finishAtom, resolveFun, js::frontend::NameFunctions, js_arena_malloc
[task 2019-02-01T05:37:59.004Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame js::StringBuffer::finishAtom matched a expected leak
[task 2019-02-01T05:37:59.005Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at js::ScriptDecodeTask::parse, mozilla::PeerConnectionMedia::AddTransceiver, CreateTransceiverImpl, mozilla::PeerConnectionImpl::SetRemoteDescription
[task 2019-02-01T05:37:59.005Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame mozilla::PeerConnectionMedia::AddTransceiver matched a expected leak
[task 2019-02-01T05:37:59.007Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at js::RunScript, js::ExecuteKernel
[task 2019-02-01T05:37:59.007Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame js::RunScript matched a expected leak
[task 2019-02-01T05:37:59.008Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at mozilla::dom::RTCPeerConnectionJSImpl::__Init, mozilla::dom::RTCPeerConnection::Constructor, mozilla::dom::RTCPeerConnection_Binding::_constructor, CallJSNative
[task 2019-02-01T05:37:59.008Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame mozilla::dom::RTCPeerConnectionJSImpl::__Init matched a expected leak
[task 2019-02-01T05:37:59.009Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at EmitAssignmentRhs, js::frontend::BytecodeEmitter::emitAssignment, js::frontend::BytecodeEmitter::emitTree, js_arena_malloc
[task 2019-02-01T05:37:59.009Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame js::frontend::BytecodeEmitter::emitAssignment matched a expected leak
[task 2019-02-01T05:37:59.009Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at js::frontend::Parser
[task 2019-02-01T05:37:59.010Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame js::frontend::Parser matched a expected leak
[task 2019-02-01T05:37:59.010Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at mozilla::dom::ChromeUtils::Import, js::frontend::GeneralParser, js::frontend::GeneralParser, js::frontend::GeneralParser
[task 2019-02-01T05:37:59.010Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame mozilla::dom::ChromeUtils::Import matched a expected leak
[task 2019-02-01T05:37:59.011Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at mozilla::EventDispatcher::DispatchDOMEvent, mozilla::dom::Document::DispatchPageTransition, mozilla::dom::Document::OnPageShow, nsDocumentViewer::LoadComplete
[task 2019-02-01T05:37:59.011Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame mozilla::EventDispatcher::DispatchDOMEvent matched a expected leak
[task 2019-02-01T05:37:59.012Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at js::CallSelfHostedFunction, AsyncFunctionResume, js_arena_malloc, js_new
[task 2019-02-01T05:37:59.012Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame js::CallSelfHostedFunction matched a expected leak
[task 2019-02-01T05:37:59.013Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at js::frontend::GeneralParser, js::frontend::GeneralParser, js::frontend::GeneralParser, AllocateProtoAndIfaceCache
[task 2019-02-01T05:37:59.013Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame js::frontend::GeneralParser matched a expected leak
[task 2019-02-01T05:37:59.015Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at js_arena_malloc, js_pod_arena_malloc, maybe_pod_malloc, js::TenuringTracer::moveSlotsToTenured
[task 2019-02-01T05:37:59.015Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame js_arena_malloc matched a expected leak
[task 2019-02-01T05:37:59.016Z] 05:37:59 INFO - TEST-FAIL | LeakSanitizer | leak at js_arena_malloc, AsyncFunctionPromiseReactionJob, PromiseReactionJob, CallJSNative
[task 2019-02-01T05:37:59.017Z] 05:37:59 INFO - INFO | LeakSanitizer | Frame js_arena_malloc matched a expected leak

Seen during webrtc/RTCPeerConnection-setRemoteDescription-pranswer.html

Component: DOM → WebRTC

There are 265 total failures in the last 7 days on linux64 asan.
I looked through a few logs and the leaks occur after different tests.

Recent failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=227128734&repo=autoland&lineNumber=98856

Nils, can you take a look or assign someone that can take a look at this?

Flags: needinfo?(drno)
Whiteboard: [stockwell disable-recommended] → [stockwell needswork:owner]
Whiteboard: [stockwell disable-recommended] → [stockwell needswork:owner]

J.C. Please take a look at the above comment.

Removing ni for Nils for now.

Flags: needinfo?(drno) → needinfo?(jjones)

I've no real idea what to do here, either; I don't see WebAuthn-y stacks in the lsan data anymore after my fixes from bug 1540378, bug 1540658, and bug 1541085 landed.

I'm also conveniently departing on paternity leave now. I certainly don't want to be remembered for clearing a needinfo I don't want to handle and just disappearing.... so I'll redirect it back to you instead, and you're welcome to publicly shame me in Whistler. Or perhaps hit me with a hot potato, in honor of me hot-potatoing this bug to you?

Flags: needinfo?(jjones) → needinfo?(apavel)
Whiteboard: [stockwell disable-recommended] → [stockwell needswork]

J.C. CONGRATULATIONS!!

Don't worry about bugs, enjoy your paternity leave to the fullest, you will not be remembered as the one who cleared ni just because of PTO.

Dana, since you reviewed the patch, would you mind taking a look at this, specifically at comment 14?

Joel, if there is no resolution here sooner, how could we disable this since it's leaking in different tests?

Flags: needinfo?(jmaher)
Flags: needinfo?(dkeeler)
Flags: needinfo?(apavel)

this is a regression in one of the tests in webauthn/ :
https://searchfox.org/mozilla-central/source/testing/web-platform/tests/webauthn

it could be >1, this is a case where we can push to try running one test at a time to find a root cause. In some cases this is an interaction between multiple tests.

if :dkeeler doesn't have a solution or is up for backing out, then needinfo me and we can try to bisect on try.

Flags: needinfo?(jmaher)

Okay Joel, thank you!

Whiteboard: [stockwell disable-recommended] → [stockwell needswork:owner]
Whiteboard: [stockwell disable-recommended] → [stockwell needswork:owner]
Whiteboard: [stockwell disable-recommended] → [stockwell needswork:owner]

I'm having a hard time reproducing these failures. Is there some configuration I need to do other than enabling asan/lsan?

Flags: needinfo?(dkeeler) → needinfo?(jmaher)

my understanding is this is just linux64 asan; possibly we need the entire set of tests in order to make this work instead of testing just one test (i.e. /webauthn/getcredential-badargs-rpid.https.html). Also this might be slightly related to the hardware environment- maybe there is a timing issue why we get leaks and locally we can clean everything up in the right order.

Have you pushed to try and done work there?

Flags: needinfo?(jmaher)

Aha - if I build with debug disabled, I can reproduce this (I assume because otherwise the JIT's disabled). Incidentally, from my reading of the logs, the test harness launches a new browser process per directory, so other tests (in other directories) shouldn't have an effect on this.

See Also: → 1543938
Assignee: nobody → dkeeler
Component: WebRTC → DOM: Web Authentication
Priority: P5 → P1

Before this patch, the WebAuthnManager/U2F destructors would call MaybeReject on
existing transaction promises. Doing this leaks JS objects. If
WebAuthnManager/U2F are being destructed, though, the window is going away, so
it shouldn't be necessary to reject any outstanding promises. This patch just
clears the transactions.

Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e851e7d1789f
don't create JS objects from WebAuthnManager or U2F destructors r=qdot
Whiteboard: [stockwell disable-recommended] → [stockwell needswork:owner]
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Can we remove the LSAN annotations from the webauthn tests now?
https://searchfox.org/mozilla-central/source/testing/web-platform/meta/webauthn/__dir__.ini

Also, is this something we should consider uplifting since it goes back to at least 59 AFAICT?

Flags: needinfo?(dkeeler)
Keywords: memory-leak

Comment on attachment 9057960 [details]
bug 1524478 - don't create JS objects from WebAuthnManager or U2F destructors r?qdot

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 1409434
  • User impact if declined: Memory leak if a user closes a page while a webauthn operation is happening in the background.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a small change that only has an effect when the page is being closed, so it shouldn't be a problem.
  • String changes made/needed:

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: It's important for the safety of the web and the future of Firefox that we have a solid webauthn implementation.
  • User impact if declined: Memory leak if a user closes a page while a webauthn operation is happening in the background.
  • Fix Landed on Version: 68
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a small change that only has an effect when the page is being closed, so it shouldn't be a problem.
  • String or UUID changes made by this patch:
Flags: needinfo?(dkeeler)
Attachment #9057960 - Flags: approval-mozilla-esr60?
Attachment #9057960 - Flags: approval-mozilla-beta?

Comment on attachment 9057960 [details]
bug 1524478 - don't create JS objects from WebAuthnManager or U2F destructors r?qdot

Low risk, approved for 67 beta 12, thanks.

Attachment #9057960 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment on attachment 9057960 [details]
bug 1524478 - don't create JS objects from WebAuthnManager or U2F destructors r?qdot

Fix for memory leak, let's take this for 60.7 ESR.

Attachment #9057960 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7a643fd33d8c
remove unnecessary lsan annotations for webauthn web-platform tests r=qdot
You need to log in before you can comment on or make changes to this bug.