Skip to content

Commit

Permalink
Fix race condition in @apollo/client/link/context that can leak subsc…
Browse files Browse the repository at this point in the history
…riptions (#8399)

Fixes #8398.
  • Loading branch information
sofianhn authored Jun 22, 2021
1 parent e516d46 commit 2e19922
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 0 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## Apollo Client 3.3.21 (not yet released)

### Bug fixes

- Fix race condition in `@apollo/client/link/context` that could leak subscriptions if the subscription is cancelled before `operation.setContext` is called. <br/>
[@sofianhn](https://github.com/sofianhn) in [#8399](https://github.com/apollographql/apollo-client/pull/8399)

## Apollo Client 3.3.20

### Bug fixes
Expand Down
33 changes: 33 additions & 0 deletions src/link/context/__tests__/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,36 @@ it('unsubscribes without throwing before data', done => {
done();
}, 10);
});

it('does not start the next link subscription if the upstream subscription is already closed', done => {
let promiseResolved = false;
const withContext = setContext(() =>
sleep(5).then(() => {
promiseResolved = true;
return { dynamicallySet: true }
}),
);

let mockLinkCalled = false;
const mockLink = new ApolloLink(operation => {
mockLinkCalled = true;
fail('link should not be called');
});

const link = withContext.concat(mockLink);

let subscriptionReturnedData = false;
let handle = execute(link, { query }).subscribe(result => {
subscriptionReturnedData = true;
fail('subscription should not return data');
});

handle.unsubscribe();

setTimeout(() => {
expect(promiseResolved).toBe(true);
expect(mockLinkCalled).toBe(false);
expect(subscriptionReturnedData).toBe(false);
done();
}, 10);
});
4 changes: 4 additions & 0 deletions src/link/context/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@ export function setContext(setter: ContextSetter): ApolloLink {

return new Observable(observer => {
let handle: ZenObservable.Subscription;
let closed = false;
Promise.resolve(request)
.then(req => setter(req, operation.getContext()))
.then(operation.setContext)
.then(() => {
// if the observer is already closed, no need to subscribe.
if (closed) return;
handle = forward(operation).subscribe({
next: observer.next.bind(observer),
error: observer.error.bind(observer),
Expand All @@ -25,6 +28,7 @@ export function setContext(setter: ContextSetter): ApolloLink {
.catch(observer.error.bind(observer));

return () => {
closed = true;
if (handle) handle.unsubscribe();
};
});
Expand Down

0 comments on commit 2e19922

Please sign in to comment.