Skip to content

fix: Fix memory leak in PlaywrightCrawler on browser context creation #1446

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Oct 1, 2025
Merged

fix: Fix memory leak in PlaywrightCrawler on browser context creation #1446

merged 6 commits into from
Oct 1, 2025

Conversation

Pijukatel
Copy link
Collaborator

@Pijukatel Pijukatel commented Oct 1, 2025 •
edited
Loading

Description

  • Add Lock to PlaywrightBrowserController._browser_context creation to ensure no orphaned duplicate contexts are created.

Issues

Testing

  • Unit test added.
  • Benchmark actor with PlaywrightCrawler used to confirm normal memory consumption with this fix.

@github-actions github-actions bot added this to the 124th sprint - Tooling team milestone Oct 1, 2025
@github-actions github-actions bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Oct 1, 2025
@Pijukatel Pijukatel changed the title fix: Fix memory leak in PlaywrightCrawler on context creation fix: Fix memory leak in PlaywrightCrawler on browser context creation Oct 1, 2025
@github-actions github-actions bot added the tested Temporary label used only programatically for some analytics. label Oct 1, 2025
@Pijukatel Pijukatel requested review from Mantisus and vdusek October 1, 2025 11:28
@Pijukatel Pijukatel force-pushed the WIP-strange-pw-context branch from 73253e8 to ef063a1 Compare October 1, 2025 11:38
@Pijukatel Pijukatel marked this pull request as ready for review October 1, 2025 11:39
@Pijukatel
Copy link
Collaborator Author

Pijukatel commented Oct 1, 2025 •
edited
Loading

Memory consumption of PlaywrightCrawler on 2000 pages:

  • With fix: Average:378.4 MB Maximum:638.8 MB
  • Without fix: Average:2.8 GBMaximum:3.8 GB -> Gets memory starved and stops.

vdusek
vdusek approved these changes Oct 1, 2025
View reviewed changes
Copy link
Collaborator

@vdusek vdusek left a comment

Choose a reason for hiding this comment

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

Good job, thanks

@Pijukatel
Copy link
Collaborator Author

Ignoring the failing test due to #1441

@Pijukatel Pijukatel merged commit bb181e5 into master Oct 1, 2025
18 of 19 checks passed
@Pijukatel Pijukatel deleted the WIP-strange-pw-context branch October 1, 2025 12:29
@B4nan
Copy link
Member

B4nan commented Oct 1, 2025

We should get this out soon, doesn't have to be right ahead, but I'd ship it this week.

@vdusek
Copy link
Collaborator

vdusek commented Oct 1, 2025

We should get this out soon, doesn't have to be right ahead, but I'd ship it this week.

Sure, ideally together with the #1438 and #1440.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vdusek vdusek vdusek approved these changes

@Mantisus Mantisus Awaiting requested review from Mantisus

Assignees

@Pijukatel Pijukatel

Labels
t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.
Projects
None yet
Milestone
124th sprint - Tooling team
Development

Successfully merging this pull request may close these issues.

PlaywrightCrawler constant memory grow
3 participants
@Pijukatel @B4nan @vdusek