corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1660555 - Let AbortSignal grab strong references to its followers r=smaug

Browse Source 
Differential Revision: https://phabricator.services.mozilla.com/D136447
This commit is contained in:
Kagami Sascha Rosylight
2022-01-21 14:40:39 +00:00
parent 5af4408463
commit e1716b5a55
4 changed files with 33 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
dom/abort/AbortFollower.h
View File

@@ -24,8 +24,13 @@ class AbortFollower : public nsISupports {
public: public:
virtual void RunAbortAlgorithm() = 0; virtual void RunAbortAlgorithm() = 0;
// This adds strong reference to this follower on the signal, which means
// you'll need to call Unfollow() to prevent your object from living
// needlessly longer.
void Follow(AbortSignalImpl* aSignal); void Follow(AbortSignalImpl* aSignal);
// Explicitly call this to let garbage collection happen sooner when the
// follower finished its work and cannot be aborted anymore.
void Unfollow(); void Unfollow();
bool IsFollowing() const; bool IsFollowing() const;
@@ -63,7 +68,7 @@ class AbortSignalImpl : public nsISupports, public SupportsWeakPtr {
static void Unlink(AbortSignalImpl* aSignal); static void Unlink(AbortSignalImpl* aSignal);
virtual ~AbortSignalImpl() = default; virtual ~AbortSignalImpl() { UnlinkFollowers(); }
JS::Heap<JS::Value> mReason; JS::Heap<JS::Value> mReason;
@@ -72,11 +77,13 @@ class AbortSignalImpl : public nsISupports, public SupportsWeakPtr {
void MaybeAssignAbortError(JSContext* aCx); void MaybeAssignAbortError(JSContext* aCx);
void UnlinkFollowers();
// Raw pointers. |AbortFollower::Follow| adds to this array, and // Raw pointers. |AbortFollower::Follow| adds to this array, and
// |AbortFollower::Unfollow| (also callbed by the destructor) will remove // |AbortFollower::Unfollow| (also called by the destructor) will remove
// from this array. Finally, calling |SignalAbort()| will (after running all // from this array. Finally, calling |SignalAbort()| will (after running all
// abort algorithms) empty this and make all contained followers |Unfollow()|. // abort algorithms) empty this and make all contained followers |Unfollow()|.
nsTObserverArray<AbortFollower*> mFollowers; nsTObserverArray<RefPtr<AbortFollower>> mFollowers;
bool mAborted; bool mAborted;
}; };

20
dom/abort/AbortSignal.cpp
View File

@@ -53,26 +53,23 @@ void AbortSignalImpl::SignalAbort(JS::Handle<JS::Value> aReason) {
// https://dom.spec.whatwg.org/#abortsignal-remove could be invoked in an // https://dom.spec.whatwg.org/#abortsignal-remove could be invoked in an
// earlier algorithm to remove a later algorithm, so |mFollowers| must be a // earlier algorithm to remove a later algorithm, so |mFollowers| must be a
// |nsTObserverArray| to defend against mutation. // |nsTObserverArray| to defend against mutation.
for (RefPtr<AbortFollower> follower : mFollowers.ForwardRange()) { for (RefPtr<AbortFollower>& follower : mFollowers.ForwardRange()) {
MOZ_ASSERT(follower->mFollowingSignal == this); MOZ_ASSERT(follower->mFollowingSignal == this);
follower->RunAbortAlgorithm(); follower->RunAbortAlgorithm();
} }
// Step 4. // Step 4.
// Clear follower->signal links, then clear signal->follower links. UnlinkFollowers();
for (AbortFollower* follower : mFollowers.ForwardRange()) {
follower->mFollowingSignal = nullptr;
}
mFollowers.Clear();
} }
void AbortSignalImpl::Traverse(AbortSignalImpl* aSignal, void AbortSignalImpl::Traverse(AbortSignalImpl* aSignal,
nsCycleCollectionTraversalCallback& cb) { nsCycleCollectionTraversalCallback& cb) {
// To be filled in shortly. ImplCycleCollectionTraverse(cb, aSignal->mFollowers, "mFollowers", 0);
} }
void AbortSignalImpl::Unlink(AbortSignalImpl* aSignal) { void AbortSignalImpl::Unlink(AbortSignalImpl* aSignal) {
aSignal->mReason.setUndefined(); aSignal->mReason.setUndefined();
aSignal->UnlinkFollowers();
} }
void AbortSignalImpl::MaybeAssignAbortError(JSContext* aCx) { void AbortSignalImpl::MaybeAssignAbortError(JSContext* aCx) {
@@ -91,6 +88,15 @@ void AbortSignalImpl::MaybeAssignAbortError(JSContext* aCx) {
mReason.set(exception); mReason.set(exception);
} }
void AbortSignalImpl::UnlinkFollowers() {
// Manually unlink all followers before destructing the array, or otherwise
// the array will be accessed by Unfollow() while being destructed.
for (RefPtr<AbortFollower>& follower : mFollowers.ForwardRange()) {
follower->mFollowingSignal = nullptr;
}
mFollowers.Clear();
}
// AbortSignal // AbortSignal
// ---------------------------------------------------------------------------- // ----------------------------------------------------------------------------

2
dom/fetch/FetchDriver.cpp
View File

@@ -909,6 +909,7 @@ void FetchDriver::FailWithNetworkError(nsresult rv) {
} }
mChannel = nullptr; mChannel = nullptr;
Unfollow();
} }
NS_IMETHODIMP NS_IMETHODIMP
@@ -1438,6 +1439,7 @@ void FetchDriver::FinishOnStopRequest(
} }
mChannel = nullptr; mChannel = nullptr;
Unfollow();
} }
NS_IMETHODIMP NS_IMETHODIMP

8
testing/web-platform/tests/fetch/api/crashtests/request.html Normal file
View File

@@ -0,0 +1,8 @@
<!DOCTYPE html>
<meta charset="utf-8">
<script src="/common/utils.js"></script>
<script>
// Cycle collection test for a case where the Request object is alive and accessible globally.
var req = new Request(`/`);
fetch(req)
</script>