Closed Bug 1553855 Opened 1 year ago Closed 10 months ago

Classifier Update thread should be a LazyIdle thread

Categories

(Toolkit :: Safe Browsing, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox69 --- wontfix
firefox71 --- wontfix
firefox72 --- fixed

People

(Reporter: jesup, Assigned: dimi)

References

Details

(Whiteboard: [memshrink:p2][geckoview:p3])

Attachments

(4 files)

The Classifier Update thread is rarely active, but is created on startup and lives until shutdown. It should be moved to a LazyIdle thread to avoid memory use for the 99.9% of the time it's not active.

The extra memory usage is more important on low-memory desktops and on Android devices.

Whiteboard: [memshrink][geckoview] → [memshrink][geckoview:p3]
Priority: -- → P2
Assignee: nobody → rjesup
Status: NEW → ASSIGNED
Pushed by rjesup@wgate.com:
https://hg.mozilla.org/integration/autoland/rev/5bb757ffb7e9
Make the Classifier Update thread a LazyIdle thread r=dimi

Backed out changeset 5bb757ffb7e9 (Bug 1553855) for Classifier.cpp assertion failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=5bb757ffb7e9f9c89cd327e06a6888548362aba4&selectedJob=248652719

Backout link: https://hg.mozilla.org/integration/autoland/rev/795082d54c998ff568d64472e8f0af828a011eae

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=248652719&repo=autoland&lineNumber=1560

[task 2019-05-28T01:36:55.143Z] 01:36:55 INFO - [1049, Main Thread] WARNING: No active window: file /builds/worker/workspace/build/src/js/xpconnect/src/XPCJSContext.cpp, line 662
[task 2019-05-28T01:36:55.718Z] 01:36:55 INFO - GECKO(1074) | [Parent 1074, Main Thread] WARNING: Failed to get base domain!: file /builds/worker/workspace/build/src/ipc/glue/BackgroundUtils.cpp, line 360
[task 2019-05-28T01:36:56.260Z] 01:36:56 INFO - GECKO(1074) | [Parent 1074, Main Thread] WARNING: Suboptimal indexes for the SQL statement 0x7f20a3425410 (http://mzl.la/1FuID0j).: file /builds/worker/workspace/build/src/storage/mozStoragePrivateHelpers.cpp, line 108
[task 2019-05-28T01:36:56.481Z] 01:36:56 INFO - GECKO(1074) | [Parent 1074, StreamTrans #3] WARNING: 'NS_FAILED(rv)', file /builds/worker/workspace/build/src/modules/libjar/nsJARChannel.cpp, line 371
[task 2019-05-28T01:36:56.498Z] 01:36:56 INFO - GECKO(1074) | [Parent 1074, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file /builds/worker/workspace/build/src/modules/libjar/nsJARChannel.cpp, line 994
[task 2019-05-28T01:36:56.516Z] 01:36:56 INFO - [1049, Main Thread] WARNING: No active window: file /builds/worker/workspace/build/src/js/xpconnect/src/XPCJSContext.cpp, line 662
[task 2019-05-28T01:36:56.717Z] 01:36:56 INFO - GECKO(1074) | [Parent 1074, StreamTrans #4] WARNING: 'NS_FAILED(rv)', file /builds/worker/workspace/build/src/modules/libjar/nsJARChannel.cpp, line 371
[task 2019-05-28T01:36:56.721Z] 01:36:56 INFO - GECKO(1074) | [Parent 1074, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file /builds/worker/workspace/build/src/modules/libjar/nsJARChannel.cpp, line 994
[task 2019-05-28T01:36:56.782Z] 01:36:56 INFO - [1049, Main Thread] WARNING: No active window: file /builds/worker/workspace/build/src/js/xpconnect/src/XPCJSContext.cpp, line 662
[task 2019-05-28T01:36:57.729Z] 01:36:57 INFO - GECKO(1074) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to /tmp/tmpbFoHwO.mozrunner/runtests_leaks_tab_pid1208.log
[task 2019-05-28T01:36:57.936Z] 01:36:57 INFO - GECKO(1074) | Assertion failure: NS_GetCurrentThread() == self->mUpdateThread (MUST be on update thread), at /builds/worker/workspace/build/src/toolkit/components/url-classifier/Classifier.cpp:690
[task 2019-05-28T01:37:36.198Z] 01:37:36 INFO - GECKO(1074) | #01: nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:1165]
[task 2019-05-28T01:37:36.198Z] 01:37:36 INFO -
[task 2019-05-28T01:37:36.199Z] 01:37:36 INFO - GECKO(1074) | #02: NS_ProcessNextEvent(nsIThread*, bool) [xpcom/threads/nsThreadUtils.cpp:486]
[task 2019-05-28T01:37:36.199Z] 01:37:36 INFO -
[task 2019-05-28T01:37:36.199Z] 01:37:36 INFO - GECKO(1074) | #03: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:304]
[task 2019-05-28T01:37:36.199Z] 01:37:36 INFO -
[task 2019-05-28T01:37:36.199Z] 01:37:36 INFO - GECKO(1074) | #04: MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:315]
[task 2019-05-28T01:37:36.199Z] 01:37:36 INFO -
[task 2019-05-28T01:37:36.199Z] 01:37:36 INFO - GECKO(1074) | #05: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:291]
[task 2019-05-28T01:37:36.199Z] 01:37:36 INFO -
[task 2019-05-28T01:37:36.199Z] 01:37:36 INFO - GECKO(1074) | #06: nsThread::ThreadFunc(void*) [xpcom/threads/nsThread.cpp:457]
[task 2019-05-28T01:37:36.199Z] 01:37:36 INFO -
[task 2019-05-28T01:37:36.362Z] 01:37:36 INFO - GECKO(1074) | #07: _pt_root [nsprpub/pr/src/pthreads/ptthread.c:204]
[task 2019-05-28T01:37:36.362Z] 01:37:36 INFO -
[task 2019-05-28T01:37:36.363Z] 01:37:36 INFO - GECKO(1074) | #08: libpthread.so.0 + 0x76ba
[task 2019-05-28T01:37:36.363Z] 01:37:36 INFO -
[task 2019-05-28T01:37:36.363Z] 01:37:36 INFO - GECKO(1074) | #09: libc.so.6 + 0x10741d
[task 2019-05-28T01:37:36.363Z] 01:37:36 INFO -
[task 2019-05-28T01:37:36.363Z] 01:37:36 INFO - GECKO(1074) | #10: ??? (???:???)
[task 2019-05-28T01:37:36.363Z] 01:37:36 INFO - GECKO(1074) | ExceptionHandler::GenerateDump cloned child 1221
[task 2019-05-28T01:37:36.364Z] 01:37:36 INFO - GECKO(1074) | ExceptionHandler::SendContinueSignalToChild sent continue signal to child
[task 2019-05-28T01:37:36.366Z] 01:37:36 INFO - GECKO(1074) | ExceptionHandler::WaitForContinueSignal waiting for continue signal...
[task 2019-05-28T01:37:36.367Z] 01:37:36 INFO - GECKO(1074) | Couldn't convert chrome URL: chrome://branding/locale/brand.properties
[task 2019-05-28T01:37:36.368Z] 01:37:36 INFO - GECKO(1074) | [Child 1208, Chrome_ChildThread] WARNING: pipe error (42): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 358
[task 2019-05-28T01:37:36.369Z] 01:37:36 INFO - GECKO(1074) | [Child 1208, Chrome_ChildThread] WARNING: pipe error (41): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 358
[task 2019-05-28T01:37:36.370Z] 01:37:36 INFO - GECKO(1074) | [Child 1208, Chrome_ChildThread] WARNING: pipe error (3): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 358
[task 2019-05-28T01:37:36.371Z] 01:37:36 INFO - GECKO(1074) | Exiting due to channel error.
[task 2019-05-28T01:37:36.372Z] 01:37:36 INFO - GECKO(1074) | [GFX1-]: Receive IPC close with reason=AbnormalShutdown
[task 2019-05-28T01:37:36.372Z] 01:37:36 INFO - GECKO(1074) | [Child 1155, Chrome_ChildThread] WARNING: pipe error (3): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 358
[task 2019-05-28T01:37:36.373Z] 01:37:36 INFO - GECKO(1074) | Exiting due to channel error.
[task 2019-05-28T01:37:36.374Z] 01:37:36 INFO - GECKO(1074) | [GFX1-]: Receive IPC close with reason=AbnormalShutdown
[task 2019-05-28T01:37:36.374Z] 01:37:36 INFO - GECKO(1074) | [Child 1134, Chrome_ChildThread] WARNING: pipe error (3): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 358
[task 2019-05-28T01:37:36.375Z] 01:37:36 INFO - GECKO(1074) | Exiting due to channel error.
[task 2019-05-28T01:39:47.189Z] 01:39:47 INFO - runtests.py | Waiting for browser...
[task 2019-05-28T01:39:47.190Z] 01:39:47 INFO - TEST-INFO | Main app process: exit 11
[task 2019-05-28T01:39:47.191Z] 01:39:47 ERROR - TEST-UNEXPECTED-FAIL | ShutdownLeaks | process() called before end of test suite
[task 2019-05-28T01:39:47.192Z] 01:39:47 INFO - Buffered messages finished
[task 2019-05-28T01:39:47.193Z] 01:39:47 ERROR - TEST-UNEXPECTED-FAIL | automation.py | application terminated with exit code 11
[task 2019-05-28T01:39:47.194Z] 01:39:47 INFO - runtests.py | Application ran for: 0:03:00.109465

Flags: needinfo?(rjesup)

This makes the on-thread checks work with any nsIThread-type; this should work with a SharedThreadPool in the future if we move it to that.

Flags: needinfo?(rjesup)
Whiteboard: [memshrink][geckoview:p3] → [memshrink:p2][geckoview:p3]
Pushed by rjesup@wgate.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/46cfc6e018ba
Make the Classifier Update thread a LazyIdle thread r=dimi

Backed out for assertion failures on LazyIdleThread.cpp

backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/ff89e5a0dc662ab150000d427da4b744e6575064

push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=46cfc6e018bada56fff63c6ab6a000b31ba86437&selectedJob=249115930

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=249115930&repo=mozilla-inbound&lineNumber=26226

05:42:50 INFO - TEST-START | UrlClassifierFailUpdate.CheckTableReset
05:42:50 INFO - Assertion failure: mOwningEventTarget->IsOnCurrentThread(), at z:/build/build/src/xpcom/threads/LazyIdleThread.cpp:364
05:43:09 INFO - #01: mozilla::safebrowsing::Classifier::AsyncApplyUpdates(nsTArray<RefPtr<mozilla::safebrowsing::TableUpdate> > const &,std::function<void > const &) [toolkit/components/url-classifier/Classifier.cpp:730]
05:43:09 INFO - #02: nsresult mozilla::detail::RunnableFunction<`lambda at z:/build/build/src/toolkit/components/url-classifier/tests/gtest/Common.cpp:51:72'>::Run() [xpcom/threads/nsThreadUtils.h:562]
05:43:09 INFO - #03: nsThread::ProcessNextEvent(bool,bool *) [xpcom/threads/nsThread.cpp:1165]
05:43:09 INFO - #04: NS_ProcessNextEvent(nsIThread *,bool) [xpcom/threads/nsThreadUtils.cpp:486]
05:43:09 INFO - #05: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate *) [ipc/glue/MessagePump.cpp:303]
05:43:09 INFO - #06: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:309]
05:43:09 INFO - #07: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:291]
05:43:09 INFO - #08: nsThread::ThreadFunc(void *) [xpcom/threads/nsThread.cpp:457]
05:43:10 INFO - #09: PR_NativeRunThread [nsprpub/pr/src/threads/combined/pruthr.c:406]
05:43:10 INFO - #10: static unsigned int pr_root(void *) [nsprpub/pr/src/md/windows/w95thred.c:138]
05:43:10 INFO - #11: ucrtbase.dll + 0x1c4be
05:43:10 INFO - #12: KERNEL32.DLL + 0x13034
05:43:10 INFO - #13: static void patched_BaseThreadInitThunk(int, void *, void *) [mozglue/build/WindowsDllBlocklist.cpp:701]
05:43:10 INFO - #14: ntdll.dll + 0x71461
05:43:10 INFO - mozcrash INFO | Copy/paste: Z:\task_1559193970\build\win32-minidump_stackwalk.exe Z:\task_1559193970\build\tests\gtest\347314e5-af4d-4ded-8472-a88e9255ad22.dmp Z:\task_1559193970\build\symbols
05:43:21 INFO - mozcrash INFO | Saved minidump as Z:\task_1559193970\build\blobber_upload_dir\347314e5-af4d-4ded-8472-a88e9255ad22.dmp
05:43:21 INFO - mozcrash INFO | Saved app info as Z:\task_1559193970\build\blobber_upload_dir\347314e5-af4d-4ded-8472-a88e9255ad22.extra
05:43:21 WARNING - PROCESS-CRASH | gtest | application crashed [@ mozilla::LazyIdleThread::Dispatch(already_AddRefed<nsIRunnable>,unsigned int)]

Other failure examples:

Flags: needinfo?(rjesup)

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:jesup, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(rjesup)

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #11)

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:jesup, could you have a look please?
For more information, please visit auto_nag documentation.

I'll probably take a look at this next week.

Flags: needinfo?(rjesup)
Flags: needinfo?(dlee)

I am fixing the testcase errors after introducing LazyIdleThread. Steal this bug from :jesup

Assignee: rjesup → dlee
Flags: needinfo?(dlee)

Safe Browsing update thread wakes up every 30 mins to update tables from google
and 60 mins to update tables from mozilla.

Since the update thread doesn't have always to be alive, we change the
update thread to be a LazyIdle thread instead.

A LazyIdle thread should be created and removed by the same thread. This
patch fixes testcases that trigger the assertion.

Depends on D49874

Pushed by dlee@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0f506810a0a9
P1. Make the Classifier Update thread a LazyIdle thread. r=gcp
https://hg.mozilla.org/integration/autoland/rev/c39004cf92c4
P2. Fix Safe Browsing testcase errors after introducing LazyIdle thread. r=gcp
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

firefox71=wontfix because I assume we don't need to uplift this change to GeckoView Beta (71).

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