Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dashboard] fix Memory leak when reset causes by-value panel to be deleted #161394

Merged
merged 2 commits into from
Jul 7, 2023

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Jul 6, 2023

Closes #161310

Issue caused by setState call after async action without checking that component is still mounted. Resolved issue by checking component is mounted after any async action or subscription.

@nreese nreese added release_note:skip Skip the PR/issue when compiling release notes v8.9.0 v8.10.0 Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Feature:Embeddables Relating to the Embeddable system labels Jul 6, 2023
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
embeddable 75.9KB 76.0KB +44.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 14 16 +2
securitySolution 410 414 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 15 17 +2
securitySolution 489 493 +4
total +6

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@nreese nreese marked this pull request as ready for review July 6, 2023 21:48
@nreese nreese requested a review from a team as a code owner July 6, 2023 21:48
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@ThomThomson ThomThomson self-requested a review July 7, 2023 14:39
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! though this may conflict with #159837 because the embeddable panel file has been totally re-structured in that PR. (and has the early return!)

That said, it's worth it to merge this, because I won't be backporting that PR!

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Jul 7, 2023
@nreese nreese merged commit d9c0c55 into elastic:main Jul 7, 2023
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 7, 2023
…leted (elastic#161394)

Closes elastic#161310

Issue caused by `setState` call after async action without checking that
component is still mounted. Resolved issue by checking component is
mounted after any async action or subscription.

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit d9c0c55)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.9

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jul 7, 2023
… be deleted (#161394) (#161467)

# Backport

This will backport the following commits from `main` to `8.9`:
- [[dashboard] fix Memory leak when reset causes by-value panel to be
deleted (#161394)](#161394)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Nathan
Reese","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-07-07T14:53:50Z","message":"[dashboard]
fix Memory leak when reset causes by-value panel to be deleted
(#161394)\n\nCloses
https://github.com/elastic/kibana/issues/161310\r\n\r\nIssue caused by
`setState` call after async action without checking that\r\ncomponent is
still mounted. Resolved issue by checking component is\r\nmounted after
any async action or
subscription.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"d9c0c554f8e54068c5fd8d0c27094f0d04c6dac3","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Embedding","Team:Presentation","release_note:skip","v8.9.0","v8.10.0","Feature:Embeddables"],"number":161394,"url":"https://github.com/elastic/kibana/pull/161394","mergeCommit":{"message":"[dashboard]
fix Memory leak when reset causes by-value panel to be deleted
(#161394)\n\nCloses
https://github.com/elastic/kibana/issues/161310\r\n\r\nIssue caused by
`setState` call after async action without checking that\r\ncomponent is
still mounted. Resolved issue by checking component is\r\nmounted after
any async action or
subscription.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"d9c0c554f8e54068c5fd8d0c27094f0d04c6dac3"}},"sourceBranch":"main","suggestedTargetBranches":["8.9"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/161394","number":161394,"mergeCommit":{"message":"[dashboard]
fix Memory leak when reset causes by-value panel to be deleted
(#161394)\n\nCloses
https://github.com/elastic/kibana/issues/161310\r\n\r\nIssue caused by
`setState` call after async action without checking that\r\ncomponent is
still mounted. Resolved issue by checking component is\r\nmounted after
any async action or
subscription.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"d9c0c554f8e54068c5fd8d0c27094f0d04c6dac3"}}]}]
BACKPORT-->

Co-authored-by: Nathan Reese <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Embeddables Relating to the Embeddable system Feature:Embedding Embedding content via iFrame release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.9.0 v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dashboard] Memory leak when reset causes by-value panel to be deleted
5 participants