Closed Bug 1637648 Opened 2 years ago Closed 2 years ago

[socket process] Forward observer notifications in nsHttpHandler to socket process

Categories

(Core :: Networking, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox79 --- fixed

People

(Reporter: kershaw, Assigned: kershaw)

Details

(Whiteboard: [necko-triaged])

Attachments

(5 files)

Currently, the code in nsHttpHandler::Observe() is not executed in socket process. We need to forward the observer notifications to socket process, like what we did for prefs change.

I hit an assertion on try.

[task 2020-06-09T09:06:54.050Z] 09:06:54     INFO -  PID 1167 | Assertion failure: mConfirmer, at /builds/worker/checkouts/gecko/netwerk/dns/TRRService.cpp:978
[task 2020-06-09T09:06:54.050Z] 09:06:54     INFO -  Initializing stack-fixing for the first stack frame, this may take a while...
[task 2020-06-09T09:07:02.819Z] 09:07:02     INFO -  PID 1167 | #01: non-virtual thunk to mozilla::net::TRRService::CompleteLookup(nsHostRecord*, nsresult, mozilla::net::AddrInfo*, bool, nsTSubstring<char> const&) [netwerk/dns/TRRService.cpp:0]
[task 2020-06-09T09:07:02.819Z] 09:07:02     INFO -  PID 1167 | #02: mozilla::net::TRR::FailData(nsresult) [netwerk/dns/TRR.cpp:1273]
[task 2020-06-09T09:07:02.820Z] 09:07:02     INFO -  PID 1167 | #03: mozilla::net::TRR::OnStopRequest(nsIRequest*, nsresult) [netwerk/dns/TRR.cpp:1402]
[task 2020-06-09T09:07:02.820Z] 09:07:02     INFO -  PID 1167 | #04: mozilla::net::TRRServiceChannel::OnStopRequest(nsIRequest*, nsresult) [netwerk/protocol/http/TRRServiceChannel.cpp:1186]
[task 2020-06-09T09:07:02.822Z] 09:07:02     INFO -  PID 1167 | #05: non-virtual thunk to mozilla::net::TRRServiceChannel::OnStopRequest(nsIRequest*, nsresult) [netwerk/protocol/http/TRRServiceChannel.cpp:0]
[task 2020-06-09T09:07:02.822Z] 09:07:02     INFO -  PID 1167 | #06: nsInputStreamPump::OnStateStop() [netwerk/base/nsInputStreamPump.cpp:687]
[task 2020-06-09T09:07:02.823Z] 09:07:02     INFO -  PID 1167 | #07: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) [netwerk/base/nsInputStreamPump.cpp:443]
[task 2020-06-09T09:07:02.823Z] 09:07:02     INFO -  PID 1167 | #08: non-virtual thunk to nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) [netwerk/base/nsInputStreamPump.cpp:0]

The detail of this assertion is described below.

  1. test_trr.js is finished, so we reset the pref network.trr.confirmationNS to example.com.
  2. Then, profile-change-net-teardown notification is also sent to socket process.
  3. In socket process, mConfirmationState is reset to CONFIRM_TRYING.
  4. An TRR request is failed because of profile-change-net-teardown notification.
  5. TRRService::CompleteLookup() is called and we hit the assertion since mConfirmer is not created.

To fix this, I think we should call MaybeConfirm() when network.trr.confirmationNS is changed.

(In reply to Kershaw Chang [:kershaw] from comment #4)

  1. TRRService::CompleteLookup() is called and we hit the assertion since mConfirmer is not created.

I'm not sure I understand. Why is TRRService::CompleteLookup called if mConfirmer is null?

Flags: needinfo?(kershaw)

(In reply to Valentin Gosu [:valentin] (he/him) from comment #7)

(In reply to Kershaw Chang [:kershaw] from comment #4)

  1. TRRService::CompleteLookup() is called and we hit the assertion since mConfirmer is not created.

I'm not sure I understand. Why is TRRService::CompleteLookup called if mConfirmer is null?

Because it's called by another TRR request.
From the log, I see TRRService::CompleteLookup is called by the TRR request created at here.

[task 2020-06-09T09:06:53.934Z] 09:06:53     INFO -  PID 1167 | [Socket 1181: Main Thread]: D/nsHostResolver Checking blacklist for host [ipv6.host.com], host record [0x7f6050189160].
[task 2020-06-09T09:06:53.934Z] 09:06:53     INFO -  PID 1167 | [Socket 1181: Main Thread]: D/nsHostResolver TRR blacklist ipv6.host.com
[task 2020-06-09T09:06:53.935Z] 09:06:53     INFO -  PID 1167 | [Socket 1181: Main Thread]: D/nsHostResolver Checking if host [host.com] is blacklisted
[task 2020-06-09T09:06:53.935Z] 09:06:53     INFO -  PID 1167 | [Socket 1181: Main Thread]: D/nsHostResolver TRR: verify if 'host.com' resolves as NS
[task 2020-06-09T09:06:53.936Z] 09:06:53     INFO -  PID 1167 | [Socket 1181: Main Thread]: D/nsHostResolver TRR::SendHTTPRequest resolve host.com type 2
[task 2020-06-09T09:06:53.936Z] 09:06:53     INFO -  PID 1167 | [Socket 1181: Main Thread]: V/nsHttp Creating HttpBaseChannel @0x7f6050125800
[task 2020-06-09T09:06:53.936Z] 09:06:53     INFO -  PID 1167 | [Socket 1181: Main Thread]: V/nsHttp TRRServiceChannel ctor [this=0x7f6050125800]
[task 2020-06-09T09:06:53.937Z] 09:06:53     INFO -  PID 1167 | [Socket 1181: Main Thread]: D/nsHttp nsHttpHandler::CreateTRRServiceChannel [proxyInfo=(nil)]

Then, mConfirmationState is set to CONFIRM_TRYING.

[task 2020-06-09T09:06:54.023Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: D/nsHostResolver TRR::Observe() topic=nsPref:changed
[task 2020-06-09T09:06:54.023Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: D/nsHostResolver TRR::ReadPrefs: restart confirmationNS state

We also get profile-change-net-teardown in socket process.

[task 2020-06-09T09:06:54.027Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: D/nsHttp nsHttpHandler::Observe [topic="profile-change-net-teardown"]

The TRR request (0x7f6050125800) is failed with the error code NS_ERROR_NET_RESET.

[task 2020-06-09T09:06:54.046Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: V/nsHttp TRRServiceChannel::OnStartRequest [this=0x7f6050125800 request=0x7f60501c5c40 status=0]
[task 2020-06-09T09:06:54.046Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: V/nsHttp TRRServiceChannel::CallOnStartRequest [this=0x7f6050125800]
[task 2020-06-09T09:06:54.046Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: V/nsHttp   calling mListener->OnStartRequest [this=0x7f6050125800, listener=0x7f604f765040]
[task 2020-06-09T09:06:54.047Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: D/nsHostResolver TRR::OnStartRequest 0x7f604f765000 host.com 2
[task 2020-06-09T09:06:54.047Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: V/nsHttp TRRServiceChannel::OnStopRequest [this=0x7f6050125800 request=0x7f60501c5c40 status=804b0014]
[task 2020-06-09T09:06:54.047Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: V/nsHttp nsHttpTransaction::DeleteSelfOnConsumerThread [this=0x7f60501b8800]
[task 2020-06-09T09:06:54.048Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: V/nsHttp Destroying nsHttpTransaction @0x7f60501b8800
[task 2020-06-09T09:06:54.048Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: V/nsHttp nsHttpTransaction::RemoveDispatchedAsBlocking this=0x7f60501b8800 not blocking
[task 2020-06-09T09:06:54.048Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: V/nsHttp nsHttpTransaction 0x7f60501b8800 request context set to null in ReleaseBlockingTransaction() - was (nil)
[task 2020-06-09T09:06:54.049Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: V/nsHttp TRRServiceChannel 0x7f6050125800 calling OnStopRequest
[task 2020-06-09T09:06:54.049Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: D/nsHostResolver TRR:OnStopRequest 0x7f604f765000 host.com 2 failed=0 code=804B0014
[task 2020-06-09T09:06:54.049Z] 09:06:54     INFO -  PID 1167 | [Socket 1181: Main Thread]: D/nsHostResolver TRR:OnStopRequest 0x7f604f765000 status 804b0014 mFailed 0

At the end, TRRService::CompleteLookup is called and we hit this assertion.

Flags: needinfo?(kershaw)
Pushed by kjang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6fc6ec584f86
P1: Make nsIOService the single point to forward observer notifications to socket process r=dragana
https://hg.mozilla.org/integration/autoland/rev/b284d915dc1c
P2: Make TRRServiceParent use nsIOService to forward observer notifications r=dragana
https://hg.mozilla.org/integration/autoland/rev/db984645da67
P3: Forward observer notifications to socket process for nsHttpHandler r=dragana
https://hg.mozilla.org/integration/autoland/rev/585801742fa2
P4: Add needed observes for nsSocketTransportService r=dragana
https://hg.mozilla.org/integration/autoland/rev/34fb904b1175
P5: Create mConfirmer when network.trr.confirmationNS is changed r=valentin
You need to log in before you can comment on or make changes to this bug.