Skip to content

Fix classloader memory leak, due to ClassTag#10500

Merged
mergify[bot] merged 4 commits intoplayframework:masterfrom
jtjeferreira:workaround_classtag_leaks
Jan 25, 2021
Merged

Fix classloader memory leak, due to ClassTag#10500
mergify[bot] merged 4 commits intoplayframework:masterfrom
jtjeferreira:workaround_classtag_leaks

Conversation

@jtjeferreira
Copy link
Contributor

@jtjeferreira jtjeferreira commented Oct 26, 2020

Pull Request Checklist

Helpful things

Fixes

Fixes classloader memory leak, due to ClassTag

Purpose

Launches scripted tests with a limited metaspace size. With this change most of the tests fail. After analyzing the heap dump in yourkit profiler, one can see that ClassTag#cache is not allowing the classloaders created by play to be GCd.

The fix is to separate the classloader that loads ClassTag from the classloaders that load the rest of the application classes (libraries and user code), and then the classloaders can be GCd.

I also opened a PR in scala (scala/scala#9276), but I also managed to fix it in play-plugin. As you can see in that PR discussion this is not a problem in scala 2.13, but the scripted tests only run with scala 2.12

References

#9935
scala/scala#9276

@jtjeferreira jtjeferreira changed the title fight leaks Fix classloader memory leak, due to ClassTag Oct 26, 2020
@mkurz mkurz added this to the 2.8.4 milestone Oct 26, 2020
@ignasi35 ignasi35 modified the milestones: 2.8.4, 2.8.5, 2.8.6 Nov 4, 2020
@ennru ennru modified the milestones: 2.8.6, 2.8.7, 2.8.8 Dec 9, 2020
@mkurz
Copy link
Member

mkurz commented Jan 22, 2021

@dwijnand Since you commented on this pull request a while ago and you are much more a Scala expert than I will ever be, can you please review this pull request once more? Thanks!

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

LGTM

@mkurz
Copy link
Member

mkurz commented Jan 25, 2021

@dwijnand Thanks, I am going to merge and backport then.

@mergify mergify bot merged commit 6d07894 into playframework:master Jan 25, 2021
@mkurz
Copy link
Member

mkurz commented Jan 25, 2021

@Mergifyio backport 2.8.x

@mergify
Copy link
Contributor

mergify bot commented Jan 25, 2021

Command backport 2.8.x: success

Backports have been created

@mkurz
Copy link
Member

mkurz commented Jan 25, 2021

@Mergifyio backport 2.7.x

@mergify
Copy link
Contributor

mergify bot commented Jan 25, 2021

Command backport 2.7.x: success

Backports have been created

mergify bot added a commit that referenced this pull request Jan 25, 2021
Fix classloader memory leak, due to ClassTag (bp #10500)
mergify bot added a commit that referenced this pull request Jan 26, 2021
Fix classloader memory leak, due to ClassTag (bp #10500)
jtjeferreira added a commit to jtjeferreira/playframework that referenced this pull request Mar 24, 2021
jtjeferreira added a commit to jtjeferreira/playframework that referenced this pull request Mar 24, 2021
mergify bot pushed a commit that referenced this pull request Mar 24, 2021
(cherry picked from commit 97291ec)
@raboof raboof mentioned this pull request Aug 3, 2021
raboof added a commit to raboof/playframework that referenced this pull request Aug 3, 2021
This was reduced in playframework#10500, but perhaps a bit too tight
for the dev-mode scripted test.

Fixes playframework#10930
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.

5 participants