1870594 - Handle the no-CSCP case in StreamList::CloseAll

Status: RESOLVED FIXED

(Core :: Storage: Cache API, task)

Description

[Jens Stutte [:jstutte]]

It seems that removing entries from StreamList::mList depends on CacheStreamControlParent to be ever created and/or able to send events to the child when shutting down.

We can handle this edge case, but it is unclear if this is really happening in the wild.

Comment 1

[Jens Stutte [:jstutte]]

Attachment: Bug 1870594 - Handle the no-CSCP case in StreamList::CloseAll. r?#dom-storage-reviewers

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Jens Stutte [:jstutte] from comment #0)

It seems that removing entries from StreamList::mList depends on `CacheStreamControlParent to be ever created and/or able to send events to the child when shutting down.

We can handle this edge case, but it is unclear if this is really happening in the wild.

It seems like this could be possible if a content process crashes after actions are dispatched to the Cache API DB thread but before the results are dispatched to the requesting (PBackground) thread. This would be a relatively small window.

I think we could add a test for this situation by wedging the Cache API thread, having a content process issue some Cache API requests, then terminating the content process.

Comment 3

[Jens Stutte [:jstutte]]

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #2)

I think we could add a test for this situation by wedging the Cache API thread, having a content process issue some Cache API requests, then terminating the content process.

I assume you are suggesting to add such a test in the patch. I wonder if "wedging the Cache API thread" could be a use case for bug 1844129 (I did not look into the WIP patch there, so I might be wrong) ?

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Jens Stutte [:jstutte] from comment #3)

I assume you are suggesting to add such a test in the patch. I wonder if "wedging the Cache API thread" could be a use case for bug 1844129 (I did not look into the WIP patch there, so I might be wrong) ?

Starting to type up a sketch, I think getting the test operational will be at least a medium effort and so it makes sense to land the patch now; I'll r+ the patch now for clarity. I'll see also this bug from a new bug proposing the test.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2339ef37f586
Handle the no-CSCP case in StreamList::CloseAll. r=dom-storage-reviewers,asuth

Comment 6

[Jens Stutte [:jstutte]]

Removing Artur's ni? here. We can move any further discussion about "wedging the Cache API thread" to bug 1871215.

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/2339ef37f586

Comment 8

[Jens Stutte [:jstutte]]

Saving a query to check for hangs after patch landed.