Closed Bug 1542802 Opened 5 years ago Closed 1 year ago

Make LazyIdleThread usable from more than a single thread

Tracking

()

Status:

RESOLVED FIXED

Milestone:

112 Branch

Tracking Flags:

Tracking

Status

firefox112

---

fixed

People

(Reporter: jesup, Assigned: nika)

Details

Attachments

(4 files)

LazyIdleThread locking cleanup 5 years ago Randell Jesup [:jesup] (needinfo me) 12.18 KB, patch		Details \| Diff \| Splinter Review
LazyIdleThread - use from any thread 5 years ago Randell Jesup [:jesup] (needinfo me) 9.72 KB, patch		Details \| Diff \| Splinter Review
Bug 1542802 - Part 1: Replace LazyIdleThread with a thin wrapper around nsThreadPool, r=#xpcom-reviewers!,gstoll! 1 year ago Nika Layzell [:nika] (ni? for response) 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1542802 - Part 2: Rework the UntrustedModulesProcessor test, r=gstoll! 1 year ago Nika Layzell [:nika] (ni? for response) 48 bytes, text/x-phabricator-request		Details \| Review

Randell Jesup [:jesup] (needinfo me)

Reporter

Description

•

5 years ago

LazyIdleThread has a restriction that all uses must come from the creating thread. This is problematic when we may want to dispatch work items from multiple places (such as a threadpool). An example of this is bug 1529044, where we'd like to use a LazyIdleThread to avoid spawning multiple threads that use memory and fight over locks.

Also, in reviewing LazyIdleThread, I found comments about locking that don't match the implementation.

Randell Jesup [:jesup] (needinfo me)

Reporter

Comment 1

•

5 years ago

Attached patch LazyIdleThread locking cleanup — Details — Splinter Review

Randell Jesup [:jesup] (needinfo me)

Reporter

Comment 2

•

5 years ago

Attached patch LazyIdleThread - use from any thread — Details — Splinter Review

Nathan Froyd [:froydnj]

Comment 3

•

5 years ago

Is there any reason to keep LazyIdleThread around, with its complicated implementation, when what we really want is a off main thread target that we don't have to manage the lifetime of?

Nathan Froyd [:froydnj]

Updated

•

5 years ago

Type: defect → enhancement

Priority: -- → P3

Randell Jesup [:jesup] (needinfo me)

Reporter

Comment 4

•

2 years ago

Nika - we have ~6 users of LazyIdleThread in the tree. See comment 3

Assignee: rjesup → nobody

Flags: needinfo?(rjesup)

Flags: needinfo?(nika)

Nika Layzell [:nika] (ni? for response)

Assignee

Comment 5

•

2 years ago

I'd like to get rid of LazyIdleThread in general going forward, yes. Ideally we'd replace it with uses of taskcontroller/background task queues/etc.

Flags: needinfo?(nika)

BMO Automation

Updated

•

2 years ago

Severity: normal → S3

Nika Layzell [:nika] (ni? for response)

Assignee

Comment 6

•

1 year ago

Attached file Bug 1542802 - Part 1: Replace LazyIdleThread with a thin wrapper around nsThreadPool, r=#xpcom-reviewers!,gstoll! — Details

This does not reduce the number of threads we end up using in these
cases, but simplifies the complex implementation substantially by
relying on the idle thread lifetime management logic from nsThreadPool.

The nsISerialEventTarget implementation is maintained by wrapping the
nsThreadPool with a TaskQueue.

As a result of using these more reliable backing implementations, the
type is now also safe to use from any thread, rather than requiring
dispatches to all occur from the creator thread.

Phabricator Automation

Updated

•

1 year ago

Assignee: nobody → nika

Status: NEW → ASSIGNED

Phabricator Automation

Updated

•

1 year ago

Attachment #9314397 - Attachment description: Bug 1542802 - Replace LazyIdleThread with a thin wrapper around nsThreadPool, r=#xpcom-reviewers → Bug 1542802 - Replace LazyIdleThread with a thin wrapper around nsThreadPool, r=#xpcom-reviewers!,gstoll!

Pulsebot

Comment 7

•

1 year ago

Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/eafcabbf729b
Replace LazyIdleThread with a thin wrapper around nsThreadPool, r=jesup,gstoll

Natalia Csoregi [:nataliaCs]

Comment 8

•

1 year ago

Backed out for causing Gtest crashes.

[task 2023-02-02T21:17:48.405Z] 21:17:48  WARNING -  gtest TEST-UNEXPECTED-FAIL | gtest | timed out after 300 seconds without output
[task 2023-02-02T21:17:48.405Z] 21:17:48     INFO -  mozcrash checking Z:\task_167536943523412\build\tests\gtest for minidumps...
[task 2023-02-02T21:17:48.432Z] 21:17:48     INFO -  mozcrash INFO | Copy/paste: Z:/task_167536943523412/fetches\minidump-stackwalk\minidump-stackwalk.exe --symbols-url=https://symbols.mozilla.org/ --cyborg=C:\Users\task_167536943523412\AppData\Local\Temp\tmpthmgzhci\1fcf78d6-0b1d-4b56-98f1-82e297cb1fa6.trace Z:\task_167536943523412\build\tests\gtest\1fcf78d6-0b1d-4b56-98f1-82e297cb1fa6.dmp Z:\task_167536943523412\build\symbols
[task 2023-02-02T21:17:50.449Z] 21:17:50     INFO -  Traceback (most recent call last):
[task 2023-02-02T21:17:50.454Z] 21:17:50     INFO -    File "Z:\task_167536943523412\build\venv\lib\site-packages\mozcrash\mozcrash.py", line 448, in _generate_signature
[task 2023-02-02T21:17:50.454Z] 21:17:50     INFO -      frames = crash_json.get("crashing_thread").get("frames")
[task 2023-02-02T21:17:50.455Z] 21:17:50     INFO -  AttributeError: 'NoneType' object has no attribute 'get'
[task 2023-02-02T21:17:50.455Z] 21:17:50     INFO -  mozcrash INFO | Saved minidump as Z:\task_167536943523412\build\blobber_upload_dir\1fcf78d6-0b1d-4b56-98f1-82e297cb1fa6.dmp
[task 2023-02-02T21:17:50.456Z] 21:17:50  WARNING -  PROCESS-CRASH | gtest | application crashed [an error occurred while generating the signature: 'NoneType' object has no attribute 'get']
[task 2023-02-02T21:17:50.456Z] 21:17:50     INFO -  Crash dump filename: Z:\task_167536943523412\build\tests\gtest\1fcf78d6-0b1d-4b56-98f1-82e297cb1fa6.dmp
[task 2023-02-02T21:17:50.456Z] 21:17:50     INFO -  Operating system: Windows NT
[task 2023-02-02T21:17:50.456Z] 21:17:50     INFO -                    10.0.19041
[task 2023-02-02T21:17:50.456Z] 21:17:50     INFO -  CPU: amd64
[task 2023-02-02T21:17:50.456Z] 21:17:50     INFO -       family 6 model 106 stepping 6
[task 2023-02-02T21:17:50.456Z] 21:17:50     INFO -       8 CPUs
[task 2023-02-02T21:17:50.457Z] 21:17:50     INFO -  No crash
[task 2023-02-02T21:17:50.457Z] 21:17:50     INFO -  Process uptime: 925 seconds
[task 2023-02-02T21:17:50.457Z] 21:17:50     INFO -  Thread 0
[task 2023-02-02T21:17:50.458Z] 21:17:50     INFO -   0  ntdll.dll!NtWaitForSingleObject + 0x14
[task 2023-02-02T21:17:50.458Z] 21:17:50     INFO -       rax = 0x0000000000000004    rdx = 0x0000000000000000
[task 2023-02-02T21:17:50.458Z] 21:17:50     INFO -       rcx = 0x0000000000000204    rbx = 0x0000000000000000
[task 2023-02-02T21:17:50.458Z] 21:17:50     INFO -       rsi = 0x0000000000000000    rdi = 0x0000000000000204
[task 2023-02-02T21:17:50.458Z] 21:17:50     INFO -       rbp = 0x0000000000000001    rsp = 0x00000084d51fe8b8
[task 2023-02-02T21:17:50.458Z] 21:17:50     INFO -        r8 = 0x00007ffbfba30000     r9 = 0x00007ffbfbc25000
[task 2023-02-02T21:17:50.459Z] 21:17:50     INFO -       r10 = 0x000000000000006c    r11 = 0x00000084d51fe4f0
[task 2023-02-02T21:17:50.459Z] 21:17:50     INFO -       r12 = 0x00000084d51ff660    r13 = 0x0000000000000000
[task 2023-02-02T21:17:50.459Z] 21:17:50     INFO -       r14 = 0x0000000000000204    r15 = 0x0000000000000001
[task 2023-02-02T21:17:50.459Z] 21:17:50     INFO -       rip = 0x00007ffbfbaccdf4
[task 2023-02-02T21:17:50.459Z] 21:17:50     INFO -      Found by: given as instruction pointer in context
[task 2023-02-02T21:17:50.460Z] 21:17:50     INFO -   1  KERNELBASE.dll!WaitForSingleObjectEx + 0x8d
[task 2023-02-02T21:17:50.460Z] 21:17:50     INFO -       rbx = 0x0000000000000000    rbp = 0x0000000000000001
[task 2023-02-02T21:17:50.460Z] 21:17:50     INFO -       rsp = 0x00000084d51fe8c0    r12 = 0x00000084d51ff660
[task 2023-02-02T21:17:50.460Z] 21:17:50     INFO -       r13 = 0x0000000000000000    r14 = 0x0000000000000204
[task 2023-02-02T21:17:50.460Z] 21:17:50     INFO -       r15 = 0x0000000000000001    rip = 0x00007ffbf9451a5e
[task 2023-02-02T21:17:50.460Z] 21:17:50     INFO -      Found by: call frame info
[task 2023-02-02T21:17:50.461Z] 21:17:50     INFO -   2  firefox.exe!mozilla::LauncherMain(int&, wchar_t**, mozilla::StaticXREAppData const&) [LauncherProcessWin.cpp:eafcabbf729b6c2fecd624d4741d68a10af21859 : 520 + 0xd]
[task 2023-02-02T21:17:50.461Z] 21:17:50     INFO -       rbx = 0x0000000000000000    rbp = 0x0000000000000001
[task 2023-02-02T21:17:50.461Z] 21:17:50     INFO -       rsp = 0x00000084d51fe960    r12 = 0x00000084d51ff660
[task 2023-02-02T21:17:50.461Z] 21:17:50     INFO -       r13 = 0x0000000000000000    r14 = 0x0000000000000204
[task 2023-02-02T21:17:50.462Z] 21:17:50     INFO -       r15 = 0x0000000000000001    rip = 0x00007ff6ae50a329
[task 2023-02-02T21:17:50.462Z] 21:17:50     INFO -      Found by: call frame info
[task 2023-02-02T21:17:50.462Z] 21:17:50     INFO -   3  firefox.exe!wmain(int, wchar_t**) [nsWindowsWMain.cpp:eafcabbf729b6c2fecd624d4741d68a10af21859 : 137 + 0x1d]
[task 2023-02-02T21:17:50.462Z] 21:17:50     INFO -       rbx = 0x000000000000030e    rsi = 0x000000000000030d
[task 2023-02-02T21:17:50.462Z] 21:17:50     INFO -       rdi = 0x000001c21b10d200    rbp = 0x0000000000000000
[task 2023-02-02T21:17:50.463Z] 21:17:50     INFO -       rsp = 0x00000084d51ff610    r12 = 0x0000000000000000
[task 2023-02-02T21:17:50.463Z] 21:17:50     INFO -       r13 = 0x000001c21adc5e20    r14 = 0x000001c21b10a100
[task 2023-02-02T21:17:50.463Z] 21:17:50     INFO -       r15 = 0x0000000000000000    rip = 0x00007ff6ae5011b7
[task 2023-02-02T21:17:50.463Z] 21:17:50     INFO -      Found by: call frame info
[task 2023-02-02T21:17:50.463Z] 21:17:50     INFO -   4  firefox.exe!invoke_main() [exe_common.inl : 90]
[task 2023-02-02T21:17:50.464Z] 21:17:50     INFO -      Found by: inlining
[task 2023-02-02T21:17:50.464Z] 21:17:50     INFO -   5  firefox.exe!__scrt_common_main_seh() [exe_common.inl : 288 + 0x21]
[task 2023-02-02T21:17:50.464Z] 21:17:50     INFO -       rbx = 0x00007ffbf98507a8    rsi = 0x0000000000000000
[task 2023-02-02T21:17:50.465Z] 21:17:50     INFO -       rdi = 0x000001c21adc5e20    rbp = 0x0000000000000000
[task 2023-02-02T21:17:50.465Z] 21:17:50     INFO -       rsp = 0x00000084d51ff7f0    r12 = 0x0000000000000000
[task 2023-02-02T21:17:50.465Z] 21:17:50     INFO -       r13 = 0x0000000000000000    r14 = 0x0000000000000000
[task 2023-02-02T21:17:50.465Z] 21:17:50     INFO -       r15 = 0x0000000000000000    rip = 0x00007ff6ae5a1678
[task 2023-02-02T21:17:50.465Z] 21:17:50     INFO -      Found by: call frame info
[task 2023-02-02T21:17:50.466Z] 21:17:50     INFO -   6  kernel32.dll!BaseThreadInitThunk + 0x13
[task 2023-02-02T21:17:50.466Z] 21:17:50     INFO -       rbx = 0x0000000000000000    rsi = 0x0000000000000000
[task 2023-02-02T21:17:50.466Z] 21:17:50     INFO -       rdi = 0x0000000000000000    rbp = 0x0000000000000000
[task 2023-02-02T21:17:50.466Z] 21:17:50     INFO -       rsp = 0x00000084d51ff830    r12 = 0x0000000000000000
[task 2023-02-02T21:17:50.466Z] 21:17:50     INFO -       r13 = 0x0000000000000000    r14 = 0x0000000000000000
[task 2023-02-02T21:17:50.466Z] 21:17:50     INFO -       r15 = 0x0000000000000000    rip = 0x00007ffbf9a87034
[task 2023-02-02T21:17:50.466Z] 21:17:50     INFO -      Found by: call frame info
[task 2023-02-02T21:17:50.467Z] 21:17:50     INFO -   7  ntdll.dll!RtlUserThreadStart + 0x20
[task 2023-02-02T21:17:50.467Z] 21:17:50     INFO -       rbx = 0x0000000000000000    rbp = 0x0000000000000000
[task 2023-02-02T21:17:50.467Z] 21:17:50     INFO -       rsp = 0x00000084d51ff860    r12 = 0x0000000000000000
[task 2023-02-02T21:17:50.467Z] 21:17:50     INFO -       r13 = 0x0000000000000000    r14 = 0x0000000000000000
[task 2023-02-02T21:17:50.467Z] 21:17:50     INFO -       r15 = 0x0000000000000000    rip = 0x00007ffbfba82651
[task 2023-02-02T21:17:50.467Z] 21:17:50     INFO -      Found by: call frame info
[task 2023-02-02T21:17:50.468Z] 21:17:50     INFO -  Loaded modules:
[task 2023-02-02T21:17:50.468Z] 21:17:50     INFO -  0x7ff6ae500000 - 0x7ff6ae615fff  firefox.exe  111.0.0.8433  (main)
[task 2023-02-02T21:17:50.468Z] 21:17:50     INFO -  0x7ffbeb220000 - 0x7ffbeb9b3fff  windows.storage.dll  10.0.19041.1387
[task 2023-02-02T21:17:50.468Z] 21:17:50     INFO -  0x7ffbf1560000 - 0x7ffbf15fafff  msvcp140.dll  14.16.27033.0
[task 2023-02-02T21:17:50.468Z] 21:17:50     INFO -  0x7ffbf1de0000 - 0x7ffbf1f03fff  mozglue.dll  111.0.0.8433
[task 2023-02-02T21:17:50.469Z] 21:17:50     INFO -  0x7ffbf39d0000 - 0x7ffbf39e4fff  VCRUNTIME140.dll  14.16.27033.0
[task 2023-02-02T21:17:50.469Z] 21:17:50     INFO -  0x7ffbf8a30000 - 0x7ffbf8a3bfff  cryptbase.dll  10.0.19041.546
[task 2023-02-02T21:17:50.469Z] 21:17:50     INFO -  0x7ffbf8ac0000 - 0x7ffbf8aedfff  wldp.dll  10.0.19041.1320
[task 2023-02-02T21:17:50.469Z] 21:17:50     INFO -  0x7ffbf9140000 - 0x7ffbf924cfff  gdi32full.dll  10.0.19041.1387
[task 2023-02-02T21:17:50.469Z] 21:17:50     INFO -  0x7ffbf9250000 - 0x7ffbf92d1fff  bcryptPrimitives.dll  10.0.19041.1415
[task 2023-02-02T21:17:50.469Z] 21:17:50     INFO -  0x7ffbf9390000 - 0x7ffbf942cfff  msvcp_win.dll  10.0.19041.789
[task 2023-02-02T21:17:50.470Z] 21:17:50     INFO -  0x7ffbf9430000 - 0x7ffbf96f7fff  KERNELBASE.dll  10.0.19041.1387
[task 2023-02-02T21:17:50.470Z] 21:17:50     INFO -  0x7ffbf9760000 - 0x7ffbf985ffff  ucrtbase.dll  10.0.19041.789
[task 2023-02-02T21:17:50.470Z] 21:17:50     INFO -  0x7ffbf9860000 - 0x7ffbf9881fff  win32u.dll  10.0.19041.1387
[task 2023-02-02T21:17:50.470Z] 21:17:50     INFO -  0x7ffbf9890000 - 0x7ffbf99e5fff  crypt32.dll  10.0.19041.1320
[task 2023-02-02T21:17:50.470Z] 21:17:50     INFO -  0x7ffbf9a70000 - 0x7ffbf9b2dfff  kernel32.dll  10.0.19041.1348
[task 2023-02-02T21:17:50.471Z] 21:17:50     INFO -  0x7ffbf9b30000 - 0x7ffbf9bdcfff  SHCore.dll  10.0.19041.1387
[task 2023-02-02T21:17:50.471Z] 21:17:50     INFO -  0x7ffbf9be0000 - 0x7ffbf9c7dfff  msvcrt.dll  7.0.19041.546
[task 2023-02-02T21:17:50.472Z] 21:17:50     INFO -  0x7ffbf9e10000 - 0x7ffbf9fb0fff  user32.dll  10.0.19041.1202
[task 2023-02-02T21:17:50.472Z] 21:17:50     INFO -  0x7ffbf9fc0000 - 0x7ffbfa0e9fff  ole32.dll  10.0.19041.1202
[task 2023-02-02T21:17:50.472Z] 21:17:50     INFO -  0x7ffbfa0f0000 - 0x7ffbfa144fff  shlwapi.dll  10.0.19041.1023
[task 2023-02-02T21:17:50.472Z] 21:17:50     INFO -  0x7ffbfa150000 - 0x7ffbfa893fff  shell32.dll  10.0.19041.1387
[task 2023-02-02T21:17:50.472Z] 21:17:50     INFO -  0x7ffbfa8a0000 - 0x7ffbfa94bfff  advapi32.dll  10.0.19041.1052
[task 2023-02-02T21:17:50.472Z] 21:17:50     INFO -  0x7ffbfac90000 - 0x7ffbfacbafff  gdi32.dll  10.0.19041.1202
[task 2023-02-02T21:17:50.473Z] 21:17:50     INFO -  0x7ffbfacc0000 - 0x7ffbfaceffff  imm32.dll  10.0.19041.546
[task 2023-02-02T21:17:50.473Z] 21:17:50     INFO -  0x7ffbfacf0000 - 0x7ffbfb044fff  combase.dll  10.0.19041.1348
[task 2023-02-02T21:17:50.473Z] 21:17:50     INFO -  0x7ffbfb820000 - 0x7ffbfb949fff  rpcrt4.dll  10.0.19041.1288
[task 2023-02-02T21:17:50.473Z] 21:17:50     INFO -  0x7ffbfb950000 - 0x7ffbfb9eafff  sechost.dll  10.0.19041.1415
[task 2023-02-02T21:17:50.473Z] 21:17:50     INFO -  0x7ffbfba30000 - 0x7ffbfbc24fff  ntdll.dll  10.0.19041.1288
[task 2023-02-02T21:17:50.473Z] 21:17:50     INFO -  Unloaded modules:
[task 2023-02-02T21:17:50.474Z] 21:17:50     INFO -  Unimplemented streams encountered:
[task 2023-02-02T21:17:50.474Z] 21:17:50     INFO -  Stream 0x00000016 ProcessVmCountersStream (Official) @ 0x00001430
[task 2023-02-02T21:17:50.474Z] 21:17:50     INFO -  Stream 0x00000000 UnusedStream (Official) @ 0x00000000
[task 2023-02-02T21:17:50.474Z] 21:17:50     INFO -  Stream 0x00000015 SystemMemoryInfoStream (Official) @ 0x00001244
[task 2023-02-02T21:17:50.474Z] 21:17:50     INFO -  gtest INFO | rungtests.py exits with code 1
[task 2023-02-02T21:17:50.533Z] 21:17:50    ERROR - Return code: 1

Flags: needinfo?(nika)

Phabricator Automation

Updated

•

1 year ago

Attachment #9314397 - Attachment description: Bug 1542802 - Replace LazyIdleThread with a thin wrapper around nsThreadPool, r=#xpcom-reviewers!,gstoll! → Bug 1542802 - Part 1: Replace LazyIdleThread with a thin wrapper around nsThreadPool, r=#xpcom-reviewers!,gstoll!

Nika Layzell [:nika] (ni? for response)

Assignee

Comment 9

•

1 year ago

Attached file Bug 1542802 - Part 2: Rework the UntrustedModulesProcessor test, r=gstoll! — Details

It appears that the existing test method would only fail if a large number of
calls to GetUntrustedModulesData() were silmultaneously pending, so if the
computer was able to process all replies too quickly, it could hang forever.
This appears to have caused problems, probably due to the new logic in some
cases causing less runnable traffic on the main thread, meaning that fewer
tasks are spawned (due to the SpinEventLoopUntil check being polled less
frequently).

This new approach instead waits for the previous call to
GetUntrustedModulesData() to return before trying again.

Depends on D168017

Nika Layzell [:nika] (ni? for response)

Assignee

Updated

•

1 year ago

Flags: needinfo?(nika)

Pulsebot

Comment 10

•

1 year ago

Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2e792ccd635d
Part 1: Replace LazyIdleThread with a thin wrapper around nsThreadPool, r=jesup,gstoll
https://hg.mozilla.org/integration/autoland/rev/3269e177ef92
Part 2: Rework the UntrustedModulesProcessor test, r=gstoll

Sandor Molnar[:smolnar]

Comment 11

•

1 year ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/2e792ccd635d
https://hg.mozilla.org/mozilla-central/rev/3269e177ef92

Status: ASSIGNED → RESOLVED

Closed: 1 year ago

status-firefox112: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → 112 Branch

Randell Jesup [:jesup] (needinfo me)

Reporter

Updated

•

1 year ago

Flags: needinfo?(rjesup)

You need to log in before you can comment on or make changes to this bug.