1754031 - AsyncWait implementations should support updating flags on an existing listener

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P2)

Description

[Nika Layzell [:nika] (ni? for response)]

There are a number of checks in nsIAsyncInputStream::AsyncWait implementations which currently return an error if they are called when there is already a pending operation. This can be an issue for NS_AsyncCopy which will sometimes be re-called to re-register the existing stream with different flags (to turn on/off WAIT_CLOSURE_ONLY), potentially leading to the stream copy hanging indefinitely due to both streams thinking they are being waited on with WAIT_CLOSURE_ONLY.

This patch relaxes the relevant checks to allow the check to pass if the callback wasn't changed, allowing the flags to be updated in-place, and adds a warning if this check isn't met.

This bug is a limited subset of the scope of changes from bug 1453340, which also tried to make all nsIInputStream implementations fully consistent, and added some helper utilities for handling callback lifecycles more reliably across processes. These changes should be sufficient to allow bug 1754004 to land without resurrecting the entire patch stack.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1754031 - AsyncWait implementations should support updating flags on an existing listener, r=asuth!

This operation is often performed by nsAStreamCopier when switching
between the source and sink streams, in order to enable or disable the
WAIT_CLOSURE_ONLY flag. Failing to reset the wait flags can lead to a
NS_AsyncCopy hanging until the source or sink are closed when is
alternating between waiting on input and output streams. This patch
relaxes the incorrect checks on various input streams.

Depends on D141033

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:nika, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Waiting on other bugs in the stack to land.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2541d8face65
AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/14a5f40fc11d
AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/14a5f40fc11d

Comment 7

[Norisz Fay [:noriszfay]]

Backed out as they were linked to the backed out Bug 1754004

Backout link

Comment 8

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/d1b52a6f044c
Backed out 4 changesets (bug 1754031, bug 1759563, bug 1759572, bug 1759569) because they are linked to the backed out bug 1754004 CLOSED TREE

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/44be9709a93e
AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana

Comment 10

[Noemi Erli[:noemi_erli]]

Backed out 22 changesets (Bug 1759572, Bug 1759563, Bug 1754004, Bug 1754031, Bug 1759569, Bug 1696894) for causing multiple failures CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=BQ_9eKb_QtqAQRgYOoHw2g.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=d69647a725a1c4f4dcf6a1460ac9c32a56e7f2ad

Log: https://treeherder.mozilla.org/logviewer?job_id=377070975&repo=autoland&lineNumber=3645
https://treeherder.mozilla.org/logviewer?job_id=377069392&repo=autoland&lineNumber=3841
https://treeherder.mozilla.org/logviewer?job_id=377068595&repo=autoland&lineNumber=4833
https://treeherder.mozilla.org/logviewer?job_id=377071130&repo=autoland&lineNumber=2287
https://treeherder.mozilla.org/logviewer?job_id=377070617&repo=autoland&lineNumber=8173
https://treeherder.mozilla.org/logviewer?job_id=377068816&repo=autoland&lineNumber=37681
https://treeherder.mozilla.org/logviewer?job_id=377070241&repo=autoland&lineNumber=9079

Backout: https://hg.mozilla.org/integration/autoland/rev/36e3802d69934bed836ce1131d2de0fcb97984ae

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2835afab1ad
AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana

Comment 12

[Noemi Erli[:noemi_erli]]

Backed out 22 changesets (Bug 1696894, Bug 1759569, Bug 1754031, Bug 1759563, Bug 1759572, Bug 1754004) for causing leack failures CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=673ecd5337e13d3bac175a1f405862955f14002d&selectedTaskRun=bTfZ8n03SnKvj49Rus408A.0

Log: https://treeherder.mozilla.org/logviewer?job_id=377632893&repo=autoland&lineNumber=7392
https://treeherder.mozilla.org/logviewer?job_id=377629411&repo=autoland&lineNumber=32257

Backout: https://hg.mozilla.org/integration/autoland/rev/ce1bb7d083b9428f4a0985216baddd883c54e62a

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/839343847c86
AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana

Comment 14

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/839343847c86