1644522 - Handle leak in parent process

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, defect, P2)

Description

[(No longer employed by Mozilla) Aaron Klotz]

I'm not sure where this came from, but I'm creating this bug under IPC assuming that this is the likely culprit.

My parent process currently has hundreds of open handles to one of its content processes. I don't really know much more than that at the moment. I'll try to investigate further.

Comment 1

[Andrew McCreight (out of office until 8/21) [:mccr8]]

Do you have an about:memory report? That includes things like a count of the various actors.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: memory-report.json.gz

Comment 3

[Andrew McCreight (out of office until 8/21) [:mccr8]]

Lots of background parent channels, but 4 per content parent sounds about rightt.

86 (100.0%) -- ipc-channels
├──46 (53.49%) ── PBackgroundParent
├──13 (15.12%) ── PProfilerParent
├──10 (11.63%) ── PContentParent
├──10 (11.63%) ── PProcessHangMonitorParent

Comment 4

[Andrew McCreight (out of office until 8/21) [:mccr8]]

This seems like... a lot:

415,102 (100.0%) -- observer-service-suspect
├──410,632 (98.92%) ── referent(topic=profile-change-net-teardown)

In my own session, I have at most around 450.

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

Handle count is over 134,000.

Comment 6

[Andrew McCreight (out of office until 8/21) [:mccr8]]

"Unbounded stuff in the main process" makes me think of bug 1643690, but I don't know if they are actually related.

Comment 7

[Andrew McCreight (out of office until 8/21) [:mccr8]]

If you look in the main process in the memory report under explicit/threads, there do appear to be a kajillion DOMCacheThread, so maybe this is indeed a duplicate.

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

I think I found the culprit. I set a breakpoint on kernelbase!OpenProcess and hit a few different stacks. This one:

00 KERNELBASE!OpenProcess
01 xul!audioipc::handle_passing::duplicate_platformhandle+0x2f
02 xul!audioipc::handle_passing::{{impl}}::start_send+0x2b7
03 xul!futures::sink::{{impl}}::start_send+0x2f2
04 xul!audioipc::rpc::driver::assert_send<mut audioipc::handle_passing::FramedWithPlatformHandles<audioipc::messagestream_win::AsyncMessageStream, audioipc::codec::LengthDelimitedCodec<audioipc::messages::ClientMessage, audioipc::messages::ServerMessage>>*>+0x311
05 xul!audioipc::rpc::driver::Driver<audioipc::rpc::server::ServerHandler<audioipc_server::server::CubebServer>>::process_outgoing+0x543
06 xul!audioipc::rpc::driver::Driver<audioipc::rpc::server::ServerHandler<audioipc_server::server::CubebServer>>::send_outgoing+0x543
07 xul!audioipc::rpc::driver::{{impl}}::poll+0xd87
08 xul!futures::future::map_err::{{impl}}::poll<(),audioipc::rpc::driver::Driver<audioipc::rpc::server::ServerHandler<audioipc_server::server::CubebServer>>,closure-0>+0xde7
09 xul!std::thread::local::LocalKey<core::cell::RefCell<core::option::Option<tokio_reactor::HandlePriv>>>::try_with+0x77c
0a xul!std::thread::local::LocalKey<core::cell::RefCell<core::option::Option<tokio_reactor::HandlePriv>>>::with+0x7b0
0b xul!tokio_reactor::with_default+0x7b0
0c xul!tokio::runtime::current_thread::runtime::Runtime::enter+0x893
0d xul!tokio::runtime::current_thread::runtime::Runtime::block_on<futures::sync::oneshot::Receiver<()>>+0x900
0e xul!audioipc::core::spawn_thread::{{closure}}+0xde
0f xul!std::sys_common::backtrace::__rust_begin_short_backtrace<closure-0,()>+0x10c
10 xul!std::thread::{{impl}}::spawn_unchecked::{{closure}}::{{closure}}+0x19
11 xul!std::panic::{{impl}}::call_once+0x19
12 xul!std::panicking::try::do_call+0x19
13 xul!panic_abort::__rust_maybe_catch_panic+0x19
14 xul!std::panicking::try+0x19
15 xul!std::panic::catch_unwind+0x19
16 xul!std::thread::{{impl}}::spawn_unchecked::{{closure}}+0x41
17 xul!core::ops::function::FnOnce::call_once<closure-0,()>+0x50
18 xul!alloc::boxed::{{impl}}::call_once<(),FnOnce<()>>+0x3e
19 xul!alloc::boxed::{{impl}}::call_once+0xc
1a xul!std::sys_common::thread::start_thread+0x5e
1b xul!std::sys::windows::thread::{{impl}}::new::thread_start+0x67
1c KERNEL32!BaseThreadInitThunk+0x14
1d ntdll!RtlUserThreadStart+0x21

This function opens a process handle so that it can duplicate to that target process, but it never closes it.

Comment 9

[(No longer employed by Mozilla) Aaron Klotz]

[Tracking Requested - why for this release]: Handle leak which grows over the parent process lifetime. If the number of handles gets large enough, this could cause perf degredation in interesting ways.

Comment 10

[(No longer employed by Mozilla) Aaron Klotz]

Just to be clear on why the existing code is an issue: Calling DuplicateHandle with DUPLICATE_CLOSE_SOURCE only closes the source handle that is being duplicated; it is still the responsibility of the caller to manage the source and destination process handles.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Not something we're going to address in 77 at this point, but we should definitely try to get this fixed for 78 ahead of the next ESR.

Comment 12

[Matthew Gregan [:kinetik]]

Attachment: GitHub Pull Request

Comment 13

[Julien Cristau [:jcristau]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #11)

Not something we're going to address in 77 at this point, but we should definitely try to get this fixed for 78 ahead of the next ESR.

We're about to build the last 78 beta, where are we on this? It looks like the github PR is still awaiting review.

Comment 14

[Matthew Gregan [:kinetik]]

This is ready now. I'm preparing the uplift in bug 1646576.

Comment 15

[Danial Horton]

Maybe unrelated to this, but theres been a parent handle leak for IPC processes for years when a content process is destroyed it leaves <Non-existent Process> handles in the parent with memory and related pipelines., this can quickly scale to 30k handles in a busy session thats been left open for several days.

Comparing to Edge/Chrome, these process handles are destroyed thoroughly.

Comment 16

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Danial Horton from comment #15)

Maybe unrelated to this, but theres been a parent handle leak for IPC processes for years when a content process is destroyed it leaves <Non-existent Process> handles in the parent with memory and related pipelines., this can quickly scale to 30k handles in a busy session thats been left open for several days.

Comparing to Edge/Chrome, these process handles are destroyed thoroughly.

Please file a new bug for this issue.