diff --git a/packages/relay-runtime/store/RelayModernStore.js b/packages/relay-runtime/store/RelayModernStore.js index 14378e5893a3..bbf90f6d09ea 100644 --- a/packages/relay-runtime/store/RelayModernStore.js +++ b/packages/relay-runtime/store/RelayModernStore.js @@ -907,6 +907,23 @@ class RelayModernStore implements Store { } } + // Records read by an active subscription must survive GC even if the + // operation that originally retained them has been released. Otherwise a + // record can be collected out from under a live reader (e.g. a mounted + // useFragment whose owner query's retention was released while the + // consuming subtree is kept mounted by React ), regressing it to + // a partial read on the next store read. See + // RelayModernStore-SubscriptionGc-test.js. + // + // This is intentionally skipped under shouldRetainWithinTTL_EXPERIMENTAL: + // that mode deliberately frees an Activity-hidden subtree's data once its + // query passes the cache TTL even though the subtree stays mounted (and thus + // subscribed), and protecting subscription records here would defeat that + // time-based release. + if (!this._shouldRetainWithinTTL_EXPERIMENTAL) { + this._storeSubscriptions.markActiveSubscriptionRecords(references); + } + // NOTE: It may be tempting to use `this._recordSource.clear()` // when no references are found, but that would prevent calling // maybeResolverSubscription() on any records that have an active diff --git a/packages/relay-runtime/store/RelayStoreSubscriptions.js b/packages/relay-runtime/store/RelayStoreSubscriptions.js index a2873a57896f..e9e79ad783f5 100644 --- a/packages/relay-runtime/store/RelayStoreSubscriptions.js +++ b/packages/relay-runtime/store/RelayStoreSubscriptions.js @@ -263,6 +263,14 @@ class RelayStoreSubscriptions implements StoreSubscriptions { size(): number { return this._subscriptions.size; } + + markActiveSubscriptionRecords(references: DataIDSet): void { + this._subscriptions.forEach(subscription => { + subscription.snapshot.seenRecords.forEach(dataID => { + references.add(dataID); + }); + }); + } } module.exports = RelayStoreSubscriptions; diff --git a/packages/relay-runtime/store/RelayStoreTypes.js b/packages/relay-runtime/store/RelayStoreTypes.js index c0061a294887..ea6dc97ca589 100644 --- a/packages/relay-runtime/store/RelayStoreTypes.js +++ b/packages/relay-runtime/store/RelayStoreTypes.js @@ -437,6 +437,14 @@ export interface StoreSubscriptions { * returns the number of subscriptions */ size(): number; + + /** + * Adds every record id read by an active subscription's latest snapshot + * (`seenRecords`) into `references`. Used by the store GC so that a record a + * live reader still depends on is not collected, even if the operation that + * originally retained it has been released. + */ + markActiveSubscriptionRecords(references: DataIDSet): void; } /** diff --git a/packages/relay-runtime/store/__tests__/RelayModernStore-SubscriptionGc-test.js b/packages/relay-runtime/store/__tests__/RelayModernStore-SubscriptionGc-test.js new file mode 100644 index 000000000000..8580d38cedf2 --- /dev/null +++ b/packages/relay-runtime/store/__tests__/RelayModernStore-SubscriptionGc-test.js @@ -0,0 +1,133 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict-local + * @format + * @oncall relay + */ + +'use strict'; + +const RelayNetwork = require('../../network/RelayNetwork'); +const {graphql} = require('../../query/GraphQLTag'); +const RelayModernEnvironment = require('../RelayModernEnvironment'); +const { + createOperationDescriptor, +} = require('../RelayModernOperationDescriptor'); +const RelayModernStore = require('../RelayModernStore'); +const RelayRecordSource = require('../RelayRecordSource'); + +// Two operations reference the same User record. Only DetailQuery selects its +// profilePicture child (a separate, linked record); ListQuery selects only id/name. +const ListQuery = graphql` + query RelayModernStoreSubscriptionGcTestListQuery($id: ID!) { + node(id: $id) { + ... on User { + id + name + } + } + } +`; +const DetailQuery = graphql` + query RelayModernStoreSubscriptionGcTestQuery($id: ID!, $size: [Int]) { + node(id: $id) { + ... on User { + id + profilePicture(size: $size) { + uri + } + } + } + } +`; + +function setup() { + const store = new RelayModernStore(new RelayRecordSource(), { + gcReleaseBufferSize: 0, // evict a released operation immediately + gcScheduler: run => run(), // run GC synchronously for a deterministic test + }); + const environment = new RelayModernEnvironment({ + network: RelayNetwork.create(() => + Promise.reject(new Error('no network in test')), + ), + store, + }); + const detail = createOperationDescriptor(DetailQuery, {id: '4', size: 32}); + const list = createOperationDescriptor(ListQuery, {id: '4'}); + environment.commitPayload(detail, { + node: { + id: '4', + __typename: 'User', + profilePicture: {uri: 'http://x/4.jpg'}, + }, + }); + environment.commitPayload(list, { + node: {id: '4', __typename: 'User', name: 'Zuck'}, + }); + const hasProfilePicture = () => + Array.from(environment.getStore().getSource().getRecordIDs()).some(id => + /profilePicture/.test(id), + ); + return {environment, detail, list, hasProfilePicture}; +} + +describe('RelayModernStore garbage collection with active subscriptions', () => { + it('keeps a linked child that a live subscription still reads after its owner operation is released', () => { + const {environment, detail, list, hasProfilePicture} = setup(); + + // Both screens are mounted: the detail page and a list page that the router + // keeps alive (e.g. under React ). + const listRetain = environment.retain(list); + const detailRetain = environment.retain(detail); + + // A live fragment subscription reads profilePicture (the mounted useFragment). + const snapshot = environment.lookup(detail.fragment); + expect(snapshot.isMissingData).toBe(false); + expect( + Array.from(snapshot.seenRecords).some(id => /profilePicture/.test(id)), + ).toBe(true); + const subscription = environment.subscribe(snapshot, () => {}); + + // Navigate away from the detail page: its query retention is released. The list + // op stays retained (its subtree is kept mounted). GC runs. + detailRetain.dispose(); + + // DESIRED (post-fix): the child survives because a live subscription still + // reads it. CURRENT (bug): the profilePicture record is collected (only the + // released DetailQuery selected it), and the next read of the detail operation + // is a partial read. + expect(hasProfilePicture()).toBe(true); + const after = environment.lookup(detail.fragment); + expect(after.isMissingData).toBe(false); + expect((after.data as $FlowFixMe).node.profilePicture).toEqual({ + uri: 'http://x/4.jpg', + }); + + subscription.dispose(); + listRetain.dispose(); + }); + + it('collects the child once the subscription is disposed (no over-retention)', () => { + const {environment, detail, list, hasProfilePicture} = setup(); + + const listRetain = environment.retain(list); + const detailRetain = environment.retain(detail); + const subscription = environment.subscribe( + environment.lookup(detail.fragment), + () => {}, + ); + + // With no active subscription and no retained op selecting it, the child must + // be collected — the fix must not leak records past a subscription's lifetime. + subscription.dispose(); + detailRetain.dispose(); + + expect(hasProfilePicture()).toBe(false); + + listRetain.dispose(); + }); +}); diff --git a/packages/relay-runtime/store/__tests__/__generated__/RelayModernStoreSubscriptionGcTestListQuery.graphql.js b/packages/relay-runtime/store/__tests__/__generated__/RelayModernStoreSubscriptionGcTestListQuery.graphql.js new file mode 100644 index 000000000000..a39363fecc5d --- /dev/null +++ b/packages/relay-runtime/store/__tests__/__generated__/RelayModernStoreSubscriptionGcTestListQuery.graphql.js @@ -0,0 +1,148 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @oncall relay + * + * @generated SignedSource<> + * @flow + * @lightSyntaxTransform + */ + +/* eslint-disable */ + +'use strict'; + +/*:: +import type { ConcreteRequest, Query } from 'relay-runtime'; +export type RelayModernStoreSubscriptionGcTestListQuery$variables = { + id: string, +}; +export type RelayModernStoreSubscriptionGcTestListQuery$data = { + readonly node: ?{ + readonly id?: string, + readonly name?: ?string, + }, +}; +export type RelayModernStoreSubscriptionGcTestListQuery = { + response: RelayModernStoreSubscriptionGcTestListQuery$data, + variables: RelayModernStoreSubscriptionGcTestListQuery$variables, +}; +*/ + +var node/*: ConcreteRequest*/ = (function(){ +var v0 = [ + { + "defaultValue": null, + "kind": "LocalArgument", + "name": "id" + } +], +v1 = [ + { + "kind": "Variable", + "name": "id", + "variableName": "id" + } +], +v2 = { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "id", + "storageKey": null +}, +v3 = { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "name", + "storageKey": null +}; +return { + "fragment": { + "argumentDefinitions": (v0/*:: as any*/), + "kind": "Fragment", + "metadata": null, + "name": "RelayModernStoreSubscriptionGcTestListQuery", + "selections": [ + { + "alias": null, + "args": (v1/*:: as any*/), + "concreteType": null, + "kind": "LinkedField", + "name": "node", + "plural": false, + "selections": [ + { + "kind": "InlineFragment", + "selections": [ + (v2/*:: as any*/), + (v3/*:: as any*/) + ], + "type": "User", + "abstractKey": null + } + ], + "storageKey": null + } + ], + "type": "Query", + "abstractKey": null + }, + "kind": "Request", + "operation": { + "argumentDefinitions": (v0/*:: as any*/), + "kind": "Operation", + "name": "RelayModernStoreSubscriptionGcTestListQuery", + "selections": [ + { + "alias": null, + "args": (v1/*:: as any*/), + "concreteType": null, + "kind": "LinkedField", + "name": "node", + "plural": false, + "selections": [ + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "__typename", + "storageKey": null + }, + (v2/*:: as any*/), + { + "kind": "InlineFragment", + "selections": [ + (v3/*:: as any*/) + ], + "type": "User", + "abstractKey": null + } + ], + "storageKey": null + } + ] + }, + "params": { + "cacheID": "29a946b831550b85fa74086f452f3033", + "id": null, + "metadata": {}, + "name": "RelayModernStoreSubscriptionGcTestListQuery", + "operationKind": "query", + "text": "query RelayModernStoreSubscriptionGcTestListQuery(\n $id: ID!\n) {\n node(id: $id) {\n __typename\n ... on User {\n id\n name\n }\n id\n }\n}\n" + } +}; +})(); + +if (__DEV__) { + (node/*:: as any*/).hash = "c5898009fb78653a6ef0afd22ece6c96"; +} + +module.exports = ((node/*:: as any*/)/*:: as Query< + RelayModernStoreSubscriptionGcTestListQuery$variables, + RelayModernStoreSubscriptionGcTestListQuery$data, +>*/); diff --git a/packages/relay-runtime/store/__tests__/__generated__/RelayModernStoreSubscriptionGcTestQuery.graphql.js b/packages/relay-runtime/store/__tests__/__generated__/RelayModernStoreSubscriptionGcTestQuery.graphql.js new file mode 100644 index 000000000000..7d1eb7b6e080 --- /dev/null +++ b/packages/relay-runtime/store/__tests__/__generated__/RelayModernStoreSubscriptionGcTestQuery.graphql.js @@ -0,0 +1,173 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @oncall relay + * + * @generated SignedSource<> + * @flow + * @lightSyntaxTransform + */ + +/* eslint-disable */ + +'use strict'; + +/*:: +import type { ConcreteRequest, Query } from 'relay-runtime'; +export type RelayModernStoreSubscriptionGcTestQuery$variables = { + id: string, + size?: ?ReadonlyArray, +}; +export type RelayModernStoreSubscriptionGcTestQuery$data = { + readonly node: ?{ + readonly id?: string, + readonly profilePicture?: ?{ + readonly uri: ?string, + }, + }, +}; +export type RelayModernStoreSubscriptionGcTestQuery = { + response: RelayModernStoreSubscriptionGcTestQuery$data, + variables: RelayModernStoreSubscriptionGcTestQuery$variables, +}; +*/ + +var node/*: ConcreteRequest*/ = (function(){ +var v0 = [ + { + "defaultValue": null, + "kind": "LocalArgument", + "name": "id" + }, + { + "defaultValue": null, + "kind": "LocalArgument", + "name": "size" + } +], +v1 = [ + { + "kind": "Variable", + "name": "id", + "variableName": "id" + } +], +v2 = { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "id", + "storageKey": null +}, +v3 = { + "alias": null, + "args": [ + { + "kind": "Variable", + "name": "size", + "variableName": "size" + } + ], + "concreteType": "Image", + "kind": "LinkedField", + "name": "profilePicture", + "plural": false, + "selections": [ + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "uri", + "storageKey": null + } + ], + "storageKey": null +}; +return { + "fragment": { + "argumentDefinitions": (v0/*:: as any*/), + "kind": "Fragment", + "metadata": null, + "name": "RelayModernStoreSubscriptionGcTestQuery", + "selections": [ + { + "alias": null, + "args": (v1/*:: as any*/), + "concreteType": null, + "kind": "LinkedField", + "name": "node", + "plural": false, + "selections": [ + { + "kind": "InlineFragment", + "selections": [ + (v2/*:: as any*/), + (v3/*:: as any*/) + ], + "type": "User", + "abstractKey": null + } + ], + "storageKey": null + } + ], + "type": "Query", + "abstractKey": null + }, + "kind": "Request", + "operation": { + "argumentDefinitions": (v0/*:: as any*/), + "kind": "Operation", + "name": "RelayModernStoreSubscriptionGcTestQuery", + "selections": [ + { + "alias": null, + "args": (v1/*:: as any*/), + "concreteType": null, + "kind": "LinkedField", + "name": "node", + "plural": false, + "selections": [ + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "__typename", + "storageKey": null + }, + (v2/*:: as any*/), + { + "kind": "InlineFragment", + "selections": [ + (v3/*:: as any*/) + ], + "type": "User", + "abstractKey": null + } + ], + "storageKey": null + } + ] + }, + "params": { + "cacheID": "5dc949cc5e94493addbf7d640a4adae6", + "id": null, + "metadata": {}, + "name": "RelayModernStoreSubscriptionGcTestQuery", + "operationKind": "query", + "text": "query RelayModernStoreSubscriptionGcTestQuery(\n $id: ID!\n $size: [Int]\n) {\n node(id: $id) {\n __typename\n ... on User {\n id\n profilePicture(size: $size) {\n uri\n }\n }\n id\n }\n}\n" + } +}; +})(); + +if (__DEV__) { + (node/*:: as any*/).hash = "06f96e51af17dc4bfb2e5cecfe54672e"; +} + +module.exports = ((node/*:: as any*/)/*:: as Query< + RelayModernStoreSubscriptionGcTestQuery$variables, + RelayModernStoreSubscriptionGcTestQuery$data, +>*/);