Closed Bug 1754031 Opened 6 months ago Closed 3 months ago

AsyncWait implementations should support updating flags on an existing listener

Categories

(Core :: XPCOM, defect, P2)

defect

Tracking

()

RESOLVED FIXED
102 Branch
Tracking Status
firefox102 --- fixed

People

(Reporter: nika, Assigned: nika)

References

Details

Attachments

(1 file)

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.

See Also: 17540041453340

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

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.

Flags: needinfo?(nika)
Flags: needinfo?(bugmail)

Waiting on other bugs in the stack to land.

Flags: needinfo?(nika)
Flags: needinfo?(bugmail)
Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2541d8face65
AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana
Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/14a5f40fc11d
AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch

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

Backout link

Status: RESOLVED → REOPENED
Flags: needinfo?(nika)
Resolution: FIXED → ---
Target Milestone: 102 Branch → ---
Backout by abutkovits@mozilla.com:
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
Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/44be9709a93e
AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana
Flags: needinfo?(nika)
Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a2835afab1ad
AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana
Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/839343847c86
AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana
Flags: needinfo?(nika)
Status: REOPENED → RESOLVED
Closed: 4 months ago3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch
You need to log in before you can comment on or make changes to this bug.