Closed Bug 1632748 Opened 4 years ago Closed 4 years ago

ACTIVITY_SUBTYPE_TRANSACTION_CLOSE should be the last notification for HTTP request

Categories

(Core :: Networking, defect, P2)

defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: Honza, Unassigned)

References

Details

(Whiteboard: [necko-triaged])

DevTools Network panel is using the following two Platform Services to intercept HTTP traffic:

  • nsIHttpActivityDistributor
  • nsIObserverService

The Network panel backend is assuming (has been for a long time) that ACTIVITY_SUBTYPE_TRANSACTION_CLOSE is the last event fired for specific HTTP request. But, there have been increasing number of cases recently indicating that this isn't true anymore.

E.g. I am seeing the "http-on-stop-request" is sent after ACTIVITY_SUBTYPE_TRANSACTION_CLOSE

In case that ACTIVITY_SUBTYPE_TRANSACTION_CLOSE is not supposed to be the last event we need another event/notification informing the Network panel that particular HTTP request is fully complete and the backend can remove data structure allocated for tracking it - to release memory.

I provided simple test case + detailed STRs + a patch with some logging in this comment:
https://bugzilla.mozilla.org/show_bug.cgi?id=1445637#c3

There is currently set of bugs blocked by this issue:

  • Bug 1445637 - Requests rejected by CORS don't display properly in Network panel
  • Bug 1556451 - Requests blocked by Enhanced Tracking Protection don't display properly in Network panel
  • Bug 1555057 - Show the related Add-on for blocked resources
  • Bug 1628162 - HTTP/2 pipelining causes request selection to include multiple requests (multiselect)

Honza

Hey Nhi, could this be prioritized please?

Thanks!

Flags: needinfo?(nhnguyen)

Luca posted relevant comment in bug 1555057 comment #62

Honza

Honza, could you look into what we need to do to fix this? Thanks!

Flags: needinfo?(nhnguyen) → needinfo?(honzab.moz)

I created a patch that fixes the problem on DevTools side.

Here is Try push (Green)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e51b97763fc61b9dc4790db49a6c77263334e759&selectedTaskRun=L7HA182DRG-rE_aDjHwy_w-0

@mayhemer - is it a step in the right direction or we should rather fix something on the platform side?

P2 to match the blocked bugs.

Severity: -- → S3
Priority: -- → P2

This has to be fixed on the devtools side. There is absolutely no guarantee on the notification order from the activity observer (coming from the socket thread and the transaction) and the http-on-* notifications, coming from the http channel on the main thread. there are natural races between the two, out of our control.

Flags: needinfo?(honzab.moz)

I agree with :mayhemer, there doesn't seem to be a distinct rule that one is supposed to happen before the other - even though that may have been the case in the past.
It seems like it would be a tricky to enforce this in the platform (especially with all the socket process work that is likely to change some of these assumptions).

Honza, is it possible to change the assumptions the netmonitor makes about the events?

Flags: needinfo?(odvarko)
Whiteboard: [necko-triaged]

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

Honza, is it possible to change the assumptions the netmonitor makes about the events?

Yes, I am working on this in this patch:
https://phabricator.services.mozilla.com/D65452
bug 1555057

If that fixes the problem, we can close this report.

Honza

No longer blocks: 1555057
Depends on: 1555057
Flags: needinfo?(odvarko)

bug 1555057 landed, closing this report.

Honza

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.