Closed Bug 1637648 Opened 5 years ago Closed 5 years ago

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

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla79

Tracking Flags:

Tracking

Status

firefox79

---

fixed

People

(Reporter: kershaw, Assigned: kershaw)

Details

(Whiteboard: [necko-triaged])

Attachments

(5 files)

Bug 1637648 - P1: Make nsIOService the single point to forward observer notifications to socket process 5 years ago Kershaw Chang [:kershaw] 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1637648 - P2: Make TRRServiceParent use nsIOService to forward observer notifications 5 years ago Kershaw Chang [:kershaw] 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1637648 - P3: Forward observer notifications to socket process for nsHttpHandler 5 years ago Kershaw Chang [:kershaw] 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1637648 - P4: Add needed observes for nsSocketTransportService 5 years ago Kershaw Chang [:kershaw] 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1637648 - P5: Create mConfirmer when network.trr.confirmationNS is changed 5 years ago Kershaw Chang [:kershaw] 47 bytes, text/x-phabricator-request		Details \| Review

Kershaw Chang [:kershaw]

Assignee

Description

•

5 years ago

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.

Kershaw Chang [:kershaw]

Assignee

Comment 1

•

5 years ago

Attached file Bug 1637648 - P1: Make nsIOService the single point to forward observer notifications to socket process — Details

Depends on D77304

Kershaw Chang [:kershaw]

Assignee

Comment 2

•

5 years ago

Attached file Bug 1637648 - P2: Make TRRServiceParent use nsIOService to forward observer notifications — Details

Depends on D77305

Kershaw Chang [:kershaw]

Assignee

Comment 3

•

5 years ago

Attached file Bug 1637648 - P3: Forward observer notifications to socket process for nsHttpHandler — Details

Depends on D77306

Kershaw Chang [:kershaw]

Assignee

Comment 4

•

5 years ago

•

Edited

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.

test_trr.js is finished, so we reset the pref network.trr.confirmationNS to example.com.
Then, profile-change-net-teardown notification is also sent to socket process.
In socket process, mConfirmationState is reset to CONFIRM_TRYING.
An TRR request is failed because of profile-change-net-teardown notification.
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.

Kershaw Chang [:kershaw]

Assignee

Comment 5

•

5 years ago

Attached file Bug 1637648 - P4: Add needed observes for nsSocketTransportService — Details

Depends on D77307

Kershaw Chang [:kershaw]

Assignee

Comment 6

•

5 years ago

Attached file Bug 1637648 - P5: Create mConfirmer when network.trr.confirmationNS is changed — Details

Depends on D78885

Valentin Gosu [:valentin] (he/him)

Comment 7

•

5 years ago

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

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)

Kershaw Chang [:kershaw]

Assignee

Comment 8

•

5 years ago

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

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

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)

Pulsebot

Comment 9

•

5 years ago

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

Dorel Luca [:dluca]

Comment 10

•

5 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/6fc6ec584f86
https://hg.mozilla.org/mozilla-central/rev/b284d915dc1c
https://hg.mozilla.org/mozilla-central/rev/db984645da67
https://hg.mozilla.org/mozilla-central/rev/585801742fa2
https://hg.mozilla.org/mozilla-central/rev/34fb904b1175

Status: NEW → RESOLVED

Closed: 5 years ago

status-firefox79: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla79

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

Bugzilla

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

Categories

(Core :: Networking, task, P2)

Tracking

()

People

(Reporter: kershaw, Assigned: kershaw)

References

Details

(Whiteboard: [necko-triaged])

Crash Data

Security

(public)

User Story

Attachments

(5 files)

Description

Comment 1

Comment 2

Comment 3

Comment 4

Comment 5

Comment 6

Comment 7

Comment 8

Comment 9

Comment 10

Attachment

General

Description

File Name

Content Type