Skip to content

fix(react): memory leak because of cycle reference#1144

Merged
Yradex merged 1 commit intolynx-family:mainfrom
hzy:p/hzy/memory-leak-1
Jun 26, 2025
Merged

fix(react): memory leak because of cycle reference#1144
Yradex merged 1 commit intolynx-family:mainfrom
hzy:p/hzy/memory-leak-1

Conversation

@hzy
Copy link
Copy Markdown
Collaborator

@hzy hzy commented Jun 24, 2025

This PR tres to eliminate a memory leak by a cycle like: list(FiberElement) -> componentAtIndex -> ctx array -> full SnapshotInstance tree (by property like __parent, __nextSibling and firstChild) -> list(FiberElement, which closes the circle).

To minimize the impact on performance and behavior, only list-related logic was modified.

Summary

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Jun 24, 2025

🦋 Changeset detected

Latest commit: 0ff725d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@lynx-js/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@hzy hzy requested review from Yradex, colinaaa, gaoachao and upupming June 24, 2025 12:03
gaoachao
gaoachao previously approved these changes Jun 24, 2025
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Jun 24, 2025

CodSpeed Performance Report

Merging #1144 will not alter performance

Comparing hzy:p/hzy/memory-leak-1 (0ff725d) with main (96ed3d8)

Summary

✅ 8 untouched benchmarks

@relativeci
Copy link
Copy Markdown

relativeci bot commented Jun 24, 2025

Web Explorer

#2113 Bundle Size — 257.95KiB (0%).

0ff725d(current) vs 96ed3d8 main#2099(baseline)

Bundle metrics  Change 1 change
                 Current
#2113
     Baseline
#2099
No change  Initial JS 140.07KiB 140.07KiB
No change  Initial CSS 31.82KiB 31.82KiB
No change  Cache Invalidation 0% 0%
No change  Chunks 4 4
No change  Assets 4 4
Change  Modules 206(-0.48%) 207
No change  Duplicate Modules 15 15
No change  Duplicate Code 3.41% 3.41%
No change  Packages 4 4
No change  Duplicate Packages 0 0
Bundle size by type  no changes
                 Current
#2113
     Baseline
#2099
No change  JS 226.13KiB 226.13KiB
No change  CSS 31.82KiB 31.82KiB

Bundle analysis reportBranch hzy:p/hzy/memory-leak-1Project dashboard


Generated by RelativeCIDocumentationReport issue

@relativeci
Copy link
Copy Markdown

relativeci bot commented Jun 24, 2025

React Example

#2123 Bundle Size — 233.19KiB (+0.1%).

0ff725d(current) vs 96ed3d8 main#2109(baseline)

Bundle metrics  Change 1 change
                 Current
#2123
     Baseline
#2109
No change  Initial JS 0B 0B
No change  Initial CSS 0B 0B
Change  Cache Invalidation 37.43% 0%
No change  Chunks 0 0
No change  Assets 4 4
No change  Modules 152 152
No change  Duplicate Modules 60 60
No change  Duplicate Code 45.81% 45.81%
No change  Packages 2 2
No change  Duplicate Packages 0 0
Bundle size by type  Change 1 change Regression 1 regression
                 Current
#2123
     Baseline
#2109
No change  IMG 145.76KiB 145.76KiB
Regression  Other 87.43KiB (+0.27%) 87.2KiB

Bundle analysis reportBranch hzy:p/hzy/memory-leak-1Project dashboard


Generated by RelativeCIDocumentationReport issue

@Yradex
Copy link
Copy Markdown
Collaborator

Yradex commented Jun 24, 2025

I would prefer merging #1143 first to make sure ESLint is happy.

@hzy hzy force-pushed the p/hzy/memory-leak-1 branch 2 times, most recently from 61da817 to 0ad1d78 Compare June 25, 2025 15:43
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

This PR tres to eliminate a memory leak by a cycle like: list(FiberElement) -> componentAtIndex -> ctx array -> full SnapshotInstance tree (by property like `__parent`, `__nextSibling` and `firstChild`) -> list(FiberElement, which closes the circle).

To minimize the impact on performance and behavior, only list-related logic was modified.
@hzy hzy force-pushed the p/hzy/memory-leak-1 branch from 0ad1d78 to 0ff725d Compare June 25, 2025 15:57
@Yradex Yradex added this pull request to the merge queue Jun 26, 2025
Merged via the queue into lynx-family:main with commit e6802d7 Jun 26, 2025
58 of 60 checks passed
colinaaa pushed a commit that referenced this pull request Jun 30, 2025
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @lynx-js/qrcode-rsbuild-plugin@0.4.0

### Minor Changes

- Support "Type to search" when switching entries and schema.
([#1115](#1115))

## @lynx-js/react@0.110.1

### Patch Changes

- Fix a memory leak when using `<list/>`.
([#1144](#1144))

## @lynx-js/rspeedy@0.9.11

### Patch Changes

- Enable fine-grained control for `output.inlineScripts`
([#883](#883))

    ```ts
    type InlineChunkTestFunction = (params: {
      size: number;
      name: string;
    }) => boolean;

    type InlineChunkTest = RegExp | InlineChunkTestFunction;

    type InlineChunkConfig =
      | boolean
      | InlineChunkTest
      | { enable?: boolean | "auto"; test: InlineChunkTest };
    ```

    ```ts
    import { defineConfig } from "@lynx-js/rspeedy";

    export default defineConfig({
      output: {
        inlineScripts: ({ name, size }) => {
          return name.includes("foo") && size < 1000;
        },
      },
    });
    ```

- docs: remove chunks: 'all' in comments
([#1168](#1168))

## @lynx-js/react-rsbuild-plugin@0.10.5

### Patch Changes

- Avoid IIFE in `main-thread.js` to resolve memory leak when using
`<list />`.
([#1176](#1176))

- Enable fine-grained control for `output.inlineScripts`
([#883](#883))

    ```ts
    type InlineChunkTestFunction = (params: {
      size: number;
      name: string;
    }) => boolean;

    type InlineChunkTest = RegExp | InlineChunkTestFunction;

    type InlineChunkConfig =
      | boolean
      | InlineChunkTest
      | { enable?: boolean | "auto"; test: InlineChunkTest };
    ```

    ```ts
    import { defineConfig } from "@lynx-js/rspeedy";

    export default defineConfig({
      output: {
        inlineScripts: ({ name, size }) => {
          return name.includes("foo") && size < 1000;
        },
      },
    });
    ```

- Updated dependencies
\[[`51cb73d`](51cb73d),
[`69fb042`](69fb042),
[`a7e8b5b`](a7e8b5b)]:
    -   @lynx-js/runtime-wrapper-webpack-plugin@0.1.2
    -   @lynx-js/template-webpack-plugin@0.8.1
    -   @lynx-js/react-webpack-plugin@0.6.17
    -   @lynx-js/react-alias-rsbuild-plugin@0.10.5
    -   @lynx-js/use-sync-external-store@1.5.0
    -   @lynx-js/react-refresh-webpack-plugin@0.3.3
    -   @lynx-js/css-extract-webpack-plugin@0.6.0

## @lynx-js/web-constants@0.14.1

### Patch Changes

- feat: support BTS API `lynx.reportError` && `__SetSourceMapRelease`,
now you can use it and handle it in lynx-view error event.
([#1059](#1059))

- fix: in lynx-view all-on-ui mode, the input event of input and
textarea is triggered twice, and the first e.detail is a string, which
does not conform to the expected data format.
([#1179](#1179))

- fix: under the all-on-ui strategy, reload() will add two page
elements. ([#1147](#1147))

-   Updated dependencies \[]:
    -   @lynx-js/web-worker-rpc@0.14.1

## @lynx-js/web-core@0.14.1

### Patch Changes

- feat: support BTS API `lynx.reportError` && `__SetSourceMapRelease`,
now you can use it and handle it in lynx-view error event.
([#1059](#1059))

- fix: under the all-on-ui strategy, reload() will add two page
elements. ([#1147](#1147))

- Updated dependencies
\[[`a64333e`](a64333e),
[`7751375`](7751375),
[`b52a924`](b52a924)]:
    -   @lynx-js/web-worker-runtime@0.14.1
    -   @lynx-js/web-constants@0.14.1
    -   @lynx-js/web-mainthread-apis@0.14.1
    -   @lynx-js/web-worker-rpc@0.14.1

## @lynx-js/web-elements@0.7.7

### Patch Changes

- fix: the param `index` of list scrollToPosition function should be
`position`.
([#1135](#1135))

- fix: in lynx-view all-on-ui mode, the input event of input and
textarea is triggered twice, and the first e.detail is a string, which
does not conform to the expected data format.
([#1179](#1179))

-   Updated dependencies \[]:
    -   @lynx-js/web-elements-template@0.7.7

## @lynx-js/web-mainthread-apis@0.14.1

### Patch Changes

- fix: under the all-on-ui strategy, reload() will add two page
elements. ([#1147](#1147))

- Updated dependencies
\[[`a64333e`](a64333e),
[`7751375`](7751375),
[`b52a924`](b52a924)]:
    -   @lynx-js/web-constants@0.14.1

## @lynx-js/web-worker-runtime@0.14.1

### Patch Changes

- feat: support BTS API `lynx.reportError` && `__SetSourceMapRelease`,
now you can use it and handle it in lynx-view error event.
([#1059](#1059))

- Updated dependencies
\[[`a64333e`](a64333e),
[`7751375`](7751375),
[`b52a924`](b52a924)]:
    -   @lynx-js/web-constants@0.14.1
    -   @lynx-js/web-mainthread-apis@0.14.1
    -   @lynx-js/web-worker-rpc@0.14.1

## @lynx-js/react-webpack-plugin@0.6.17

### Patch Changes

- Enable fine-grained control for `output.inlineScripts`
([#883](#883))

    ```ts
    type InlineChunkTestFunction = (params: {
      size: number;
      name: string;
    }) => boolean;

    type InlineChunkTest = RegExp | InlineChunkTestFunction;

    type InlineChunkConfig =
      | boolean
      | InlineChunkTest
      | { enable?: boolean | "auto"; test: InlineChunkTest };
    ```

    ```ts
    import { defineConfig } from "@lynx-js/rspeedy";

    export default defineConfig({
      output: {
        inlineScripts: ({ name, size }) => {
          return name.includes("foo") && size < 1000;
        },
      },
    });
    ```

## @lynx-js/runtime-wrapper-webpack-plugin@0.1.2

### Patch Changes

- Wrap with IIFE when `output.iife: false` to avoid naming conflict.
([#1176](#1176))

## @lynx-js/template-webpack-plugin@0.8.1

### Patch Changes

- feat: `::placeholder` will be compiled to `part(input)::placeholder`,
which means you can use pseudo-element CSS to add placeholder styles to
input and textarea.
([#1158](#1158))

        // before
<input placeholder-color='red' placeholder-font-weight='bold'
placeholder-font-size='20px'>

        // after
        <input>

        input::placeholder {
          color: red;
          font-weight: bold;
          font-size: 20px;
        }

- Enable fine-grained control for `output.inlineScripts`
([#883](#883))

    ```ts
    type InlineChunkTestFunction = (params: {
      size: number;
      name: string;
    }) => boolean;

    type InlineChunkTest = RegExp | InlineChunkTestFunction;

    type InlineChunkConfig =
      | boolean
      | InlineChunkTest
      | { enable?: boolean | "auto"; test: InlineChunkTest };
    ```

    ```ts
    import { defineConfig } from "@lynx-js/rspeedy";

    export default defineConfig({
      output: {
        inlineScripts: ({ name, size }) => {
          return name.includes("foo") && size < 1000;
        },
      },
    });
    ```

## create-rspeedy@0.9.11



## @lynx-js/react-alias-rsbuild-plugin@0.10.5



## upgrade-rspeedy@0.9.11



## @lynx-js/web-core-server@0.14.1



## @lynx-js/web-elements-template@0.7.7



## @lynx-js/web-worker-rpc@0.14.1

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
hzy added a commit to hzy/lynx-stack that referenced this pull request Jul 7, 2025
This PR tres to eliminate a memory leak by a cycle like:
list(FiberElement) -> componentAtIndex -> ctx array -> full
SnapshotInstance tree (by property like `__parent`, `__nextSibling` and
`firstChild`) -> list(FiberElement, which closes the circle).

To minimize the impact on performance and behavior, only list-related
logic was modified.

<!--
  Thank you for submitting a pull request!

We appreciate the time and effort you have invested in making these
changes. Please ensure that you provide enough information to allow
others to review your pull request.

Upon submission, your pull request will be automatically assigned with
reviewers.

If you want to learn more about contributing to this project, please
visit:
https://github.com/lynx-family/lynx-stack/blob/main/CONTRIBUTING.md.
-->

<!-- Can you explain the reasoning behind implementing this change? What
problem or issue does this pull request resolve? -->

<!-- It would be helpful if you could provide any relevant context, such
as GitHub issues or related discussions. -->

<!--- Check and mark with an "x" -->

- [ ] Tests updated (or not required).
- [ ] Documentation updated (or not required).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants