eliminate the ProxyResolution and SysProxySetting threads

RESOLVED FIXED in Firefox 63

Status

()

enhancement
P3
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: froydnj, Assigned: erahm)

Tracking

(Blocks 2 bugs)

Trunk
mozilla63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

(Whiteboard: [necko-triaged] [overhead:50k])

Attachments

(2 attachments, 2 obsolete attachments)

I have these threads running in a couple of my content processes right now...but I don't actually have any proxy settings.  So they're not doing anything particularly useful, and it's not obvious to me that we need to dedicate a full-time thread to these activities anyway.  Couldn't we simply run the necessary runnables on the stream transport service or something like that?
Priority: -- → P3
Whiteboard: [necko-triaged]
Whiteboard: [necko-triaged] → [necko-triaged] [overhead:50k]
We can't get rid of the 'ProxyResolution' thread, it's a special PAC related thread that's allowed to run JS off main thread. In theory it should never be used in the content process and really should be rarely used by most users. Switching to lazy creation of the thread might make more sense.
Assignee: nobody → erahm
Status: NEW → ASSIGNED
This delays the creation of the PAC thread until we need to dispatch a
runnable to it.
Attachment #8991474 - Flags: review?(daniel)
Forgot one assert that is no longer valid.
Attachment #8991479 - Flags: review?(daniel)
Attachment #8991474 - Attachment is obsolete: true
Attachment #8991474 - Flags: review?(daniel)
Attachment #8991473 - Flags: review?(daniel) → review+
Attachment #8991479 - Flags: review?(daniel) → review+
I had to tweak this one a bit. We can't set mShutdown until after dispatching our runnables or else they get dropped. This caused a race with the off-main-thread cleanup code where mShutdown might not be set yet, so we pass that state into the runnable. On the ProxyAutoConfig side we need to set mShutdown to true by default to indicate it has never been used, setting mShutdown to false is moved to the Init function to indicate that cleanup is needed.
Attachment #8991766 - Flags: review?(daniel)
Attachment #8991479 - Attachment is obsolete: true
Comment on attachment 8991766 [details] [diff] [review]
Part 2: Lazily create ProxyResolution thread

Review of attachment 8991766 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/base/nsPACMan.cpp
@@ +381,4 @@
>    CancelExistingLoad();
>  
> +  if (mPACThread) {
> +    PostCancelPendingQ(NS_ERROR_ABORT, /*aShutdown =*/ true);

I would perhaps only question why you need to pass in arguments to a method that's only used at one place...
Attachment #8991766 - Flags: review?(daniel) → review+
(In reply to Daniel Stenberg [:bagder] - PTO, back on Aug 6 from comment #7)
> Comment on attachment 8991766 [details] [diff] [review]
> Part 2: Lazily create ProxyResolution thread
> 
> Review of attachment 8991766 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: netwerk/base/nsPACMan.cpp
> @@ +381,4 @@
> >    CancelExistingLoad();
> >  
> > +  if (mPACThread) {
> > +    PostCancelPendingQ(NS_ERROR_ABORT, /*aShutdown =*/ true);
> 
> I would perhaps only question why you need to pass in arguments to a method
> that's only used at one place...

It's used in a few places [1]. I defaulted `aShutdown` to false, so they don't show up in this patch.

[1] https://searchfox.org/mozilla-central/search?q=PostCancelPendingQ&path=
Backed out 2 changesets (bug 1448034) for GTest failures

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/3e7a177e2e84deab47b84bc56a197ce7d39d3c13

Failure push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=068bb4e7b8494d8ae82dfd1b1f22680234bf038c

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=188503623&repo=mozilla-inbound&lineNumber=1154

task 2018-07-17T05:09:05.135Z] 05:09:05     INFO -  TEST-START | TestPACMan.WhenTheDHCPClientExistsAndDHCPIsNonEmptyDHCPOptionIsUsedAsPACUri
[task 2018-07-17T05:09:05.135Z] 05:09:05     INFO -  ExceptionHandler::GenerateDump cloned child 11716
[task 2018-07-17T05:09:05.135Z] 05:09:05     INFO -  ExceptionHandler::SendContinueSignalToChild sent continue signal to child
[task 2018-07-17T05:09:05.136Z] 05:09:05     INFO -  ExceptionHandler::WaitForContinueSignal waiting for continue signal...
[task 2018-07-17T05:09:05.292Z] 05:09:05     INFO -  mozcrash INFO | Downloading symbols from: https://queue.taskcluster.net/v1/task/Tk_s5rv3R4KYUod0C4ME7A/artifacts/public/build/target.crashreporter-symbols.zip
[task 2018-07-17T05:09:10.951Z] 05:09:10     INFO -  mozcrash INFO | Copy/paste: /usr/local/bin/linux64-minidump_stackwalk /builds/worker/workspace/build/tests/gtest/7243f981-4098-00af-4936-976f5bc04f3a.dmp /tmp/tmpwKfuj5
[task 2018-07-17T05:09:19.199Z] 05:09:19     INFO -  mozcrash INFO | Saved minidump as /builds/worker/workspace/build/blobber_upload_dir/7243f981-4098-00af-4936-976f5bc04f3a.dmp
[task 2018-07-17T05:09:19.200Z] 05:09:19     INFO -  mozcrash INFO | Saved app info as /builds/worker/workspace/build/blobber_upload_dir/7243f981-4098-00af-4936-976f5bc04f3a.extra
[task 2018-07-17T05:09:19.200Z] 05:09:19  WARNING -  PROCESS-CRASH | gtest | application crashed [@ mozilla::net::TestPACMan_WhenTheDHCPClientExistsAndDHCPIsNonEmptyDHCPOptionIsUsedAsPACUri_Test::TestBody()]
[task 2018-07-17T05:09:19.201Z] 05:09:19     INFO -  Crash dump filename: /builds/worker/workspace/build/tests/gtest/7243f981-4098-00af-4936-976f5bc04f3a.dmp
[task 2018-07-17T05:09:19.202Z] 05:09:19     INFO -  Operating system: Linux
[task 2018-07-17T05:09:19.203Z] 05:09:19     INFO -                    0.0.0 Linux 4.4.0-1014-aws #14taskcluster1-Ubuntu SMP Tue Apr 3 10:27:00 UTC 2018 x86_64
[task 2018-07-17T05:09:19.203Z] 05:09:19     INFO -  CPU: x86
[task 2018-07-17T05:09:19.204Z] 05:09:19     INFO -       GenuineIntel family 6 model 62 stepping 4
[task 2018-07-17T05:09:19.205Z] 05:09:19     INFO -       4 CPUs
[task 2018-07-17T05:09:19.206Z] 05:09:19     INFO -  GPU: UNKNOWN
[task 2018-07-17T05:09:19.206Z] 05:09:19     INFO -  Crash reason:  SIGSEGV
[task 2018-07-17T05:09:19.207Z] 05:09:19     INFO -  Crash address: 0x0
[task 2018-07-17T05:09:19.208Z] 05:09:19     INFO -  Process uptime: not available
[task 2018-07-17T05:09:19.209Z] 05:09:19     INFO -  Thread 0 (crashed)
[task 2018-07-17T05:09:19.209Z] 05:09:19     INFO -   0  libxul.so!mozilla::net::TestPACMan_WhenTheDHCPClientExistsAndDHCPIsNonEmptyDHCPOptionIsUsedAsPACUri_Test::TestBody() [nsIEventTarget.h: : 37 + 0x0]
[task 2018-07-17T05:09:19.210Z] 05:09:19     INFO -      eip = 0xf1f50d47   esp = 0xfff8a240   ebp = 0xfff8a2c8   ebx = 0xf54b0000
[task 2018-07-17T05:09:19.210Z] 05:09:19     INFO -      esi = 0xe6a1f1c0   edi = 0x00000000   eax = 0xed2e7190   ecx = 0x00000000
[task 2018-07-17T05:09:19.211Z] 05:09:19     INFO -      edx = 0xe6a1f1c0   efl = 0x00210202
[task 2018-07-17T05:09:19.212Z] 05:09:19     INFO -      Found by: given as instruction pointer in context
[task 2018-07-17T05:09:19.212Z] 05:09:19     INFO -   1  libxul.so!testing::Test::Run() [gtest.cc:068bb4e7b8494d8ae82dfd1b1f22680234bf038c : 2477 + 0x21]
[task 2018-07-17T05:09:19.213Z] 05:09:19     INFO -      eip = 0xf21274a9   esp = 0xfff8a2d0   ebp = 0xfff8a308   ebx = 0xf54b0000
[task 2018-07-17T05:09:19.214Z] 05:09:19     INFO -      esi = 0xe6a1f110   edi = 0xf7143130
[task 2018-07-17T05:09:19.215Z] 05:09:19     INFO -      Found by: call frame info
[task 2018-07-17T05:09:19.215Z] 05:09:19     INFO -   2  libxul.so!testing::TestInfo::Run() [gtest.cc:068bb4e7b8494d8ae82dfd1b1f22680234bf038c : 2468 + 0x1a]
[task 2018-07-17T05:09:19.216Z] 05:09:19     INFO -      eip = 0xf212f785   esp = 0xfff8a310   ebp = 0xfff8a368   ebx = 0xf54b0000
[task 2018-07-17T05:09:19.217Z] 05:09:19     INFO -      esi = 0xf71d5d60   edi = 0xf7143130
[task 2018-07-17T05:09:19.217Z] 05:09:19     INFO -      Found by: call frame info
[task 2018-07-17T05:09:19.218Z] 05:09:19     INFO -   3  libxul.so!testing::TestCase::Run() [gtest.cc:068bb4e7b8494d8ae82dfd1b1f22680234bf038c : 2633 + 0x12]
[task 2018-07-17T05:09:19.219Z] 05:09:19     INFO -      eip = 0xf212f92d   esp = 0xfff8a370   ebp = 0xfff8a3d8   ebx = 0x00000002
[task 2018-07-17T05:09:19.219Z] 05:09:19     INFO -      esi = 0xf71ceb00   edi = 0xf71d48bc
[task 2018-07-17T05:09:19.220Z] 05:09:19     INFO -      Found by: call frame info
[task 2018-07-17T05:09:19.221Z] 05:09:19     INFO -   4  libxul.so!testing::internal::UnitTestImpl::RunAllTests() [gtest.cc:068bb4e7b8494d8ae82dfd1b1f22680234bf038c : 4689 + 0x18]
[task 2018-07-17T05:09:19.221Z] 05:09:19     INFO -      eip = 0xf212fe0d   esp = 0xfff8a3e0   ebp = 0xfff8a468   ebx = 0x00000076
[task 2018-07-17T05:09:19.222Z] 05:09:19     INFO -      esi = 0xf7143130   edi = 0xf7152d50
[task 2018-07-17T05:09:19.223Z] 05:09:19     INFO -      Found by: call frame info
[task 2018-07-17T05:09:19.223Z] 05:09:19     INFO -   5  libxul.so!testing::UnitTest::Run() [gtest.cc:068bb4e7b8494d8ae82dfd1b1f22680234bf038c : 2460 + 0x8]
[task 2018-07-17T05:09:19.224Z] 05:09:19     INFO -      eip = 0xf2130154   esp = 0xfff8a470   ebp = 0xfff8a498   ebx = 0xf54b0000
[task 2018-07-17T05:09:19.224Z] 05:09:19     INFO -      esi = 0xf7143130   edi = 0xee737d90
[task 2018-07-17T05:09:19.225Z] 05:09:19     INFO -      Found by: call frame info
[task 2018-07-17T05:09:19.226Z] 05:09:19     INFO -   6  libxul.so!mozilla::RunGTestFunc(int*, char**) [gtest.h:068bb4e7b8494d8ae82dfd1b1f22680234bf038c : 2233 + 0xd]
[task 2018-07-17T05:09:19.227Z] 05:09:19     INFO -      eip = 0xf21328f4   esp = 0xfff8a4a0   ebp = 0xfff8a508   ebx = 0xf54b0000
[task 2018-07-17T05:09:19.227Z] 05:09:19     INFO -      esi = 0xed276344   edi = 0xee737d90
[task 2018-07-17T05:09:19.228Z] 05:09:19     INFO -      Found by: call frame info
[task 2018-07-17T05:09:19.229Z] 05:09:19     INFO -   7  libxul.so!XREMain::XRE_mainStartup(bool*) [nsAppRunner.cpp:068bb4e7b8494d8ae82dfd1b1f22680234bf038c : 3947 + 0x9]
[task 2018-07-17T05:09:19.229Z] 05:09:19     INFO -      eip = 0xf1bffe9c   esp = 0xfff8a510   ebp = 0xfff8a6c8   ebx = 0xf54b0000
[task 2018-07-17T05:09:19.230Z] 05:09:19     INFO -      esi = 0x00000000   edi = 0xfff8a620
[task 2018-07-17T05:09:19.230Z] 05:09:19     INFO -      Found by: call frame info
[task 2018-07-17T05:09:19.231Z] 05:09:19     INFO -   8  libxul.so!XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) [nsAppRunner.cpp:068bb4e7b8494d8ae82dfd1b1f22680234bf038c : 4891 + 0xc]
[task 2018-07-17T05:09:19.231Z] 05:09:19     INFO -      eip = 0xf1c03305   esp = 0xfff8a6d0   ebp = 0xfff8a728   ebx = 0xf54b0000
[task 2018-07-17T05:09:19.232Z] 05:09:19     INFO -      esi = 0x00000000   edi = 0xfff8a754
[task 2018-07-17T05:09:19.233Z] 05:09:19     INFO -      Found by: call frame info
[task 2018-07-17T05:09:19.233Z] 05:09:19     INFO -   9  libxul.so!XRE_main(int, char**, mozilla::BootstrapConfig const&) [nsAppRunner.cpp:068bb4e7b8494d8ae82dfd1b1f22680234bf038c : 4998 + 0x5]
[task 2018-07-17T05:09:19.234Z] 05:09:19     INFO -      eip = 0xf1c03a5e   esp = 0xfff8a730   ebp = 0xfff8a888   ebx = 0x08081000
[task 2018-07-17T05:09:19.234Z] 05:09:19     INFO -      esi = 0xfff8a754   edi = 0xfff8a798
[task 2018-07-17T05:09:19.235Z] 05:09:19     INFO -      Found by: call frame info
[task 2018-07-17T05:09:19.236Z] 05:09:19     INFO -  10  firefox!do_main [nsBrowserApp.cpp:068bb4e7b8494d8ae82dfd1b1f22680234bf038c : 233 + 0x1a]
[task 2018-07-17T05:09:19.236Z] 05:09:19     INFO -      eip = 0x0804d4da   esp = 0xfff8a890   ebp = 0xfff8b8d8   ebx = 0x08081000
[task 2018-07-17T05:09:19.237Z] 05:09:19     INFO -      esi = 0xfff8b9d4   edi = 0x00000003
[task 2018-07-17T05:09:19.237Z] 05:09:19     INFO -      Found by: call frame info
Flags: needinfo?(erahm)
Looks like I got bitrotted by bug 356831.
I just need to update the new gtest to use `DispatchToPAC` instead of accessing the thread directly.
Flags: needinfo?(erahm)
Blocks: 1476432
https://hg.mozilla.org/mozilla-central/rev/bdba0cfc639e
https://hg.mozilla.org/mozilla-central/rev/9893cdaed08b
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.