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

Fix memory leak caused by invalid KTypeWrapper's equals method #2274

Merged
merged 2 commits into from
Apr 17, 2023

Conversation

jooohn
Copy link
Contributor

@jooohn jooohn commented Apr 16, 2023

KTypeWrapper's equals method was not implemented correctly.

This bug causes a cache miss every time here, which leads to a memory leak by registering a new serialiser every time it is looked up.

This also leads to performance degradation, since the equality check is performed linearly against each registered object that always has the same hashCode.

@jooohn jooohn changed the title Fix memory leak caused by invalid KTypeWrapper equality Fix memory leak caused by invalid KTypeWrapper equals method Apr 16, 2023
@jooohn jooohn changed the title Fix memory leak caused by invalid KTypeWrapper equals method Fix memory leak caused by invalid KTypeWrapper's equals method Apr 16, 2023
@qwwdfsad qwwdfsad requested a review from shanshin April 17, 2023 08:06
Copy link
Contributor

@shanshin shanshin left a comment

Choose a reason for hiding this comment

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

Great job, thank you for the investigation!

@sandwwraith sandwwraith merged commit bbf248e into Kotlin:dev Apr 17, 2023
xBaank pushed a commit to xBaank/kotlinx.serialization that referenced this pull request Apr 20, 2023
…n#2274)

* Fix KTypeWrapper equality bug to avoid memory leak

* Optimize imports of CachingTest.kt
xBaank added a commit to xBaank/kotlinx.serialization that referenced this pull request Apr 20, 2023
xBaank added a commit to xBaank/kotlinx.serialization that referenced this pull request Apr 20, 2023
* Revert "Adapt tests for K2 and upcoming deprecations in K1 (Kotlin#2230)"

This reverts commit f9229ef.

* Revert "Fix value class encoding in various corner cases (Kotlin#2242)"

This reverts commit 3cec2f7.

* Revert "Improved docs for JsonContentPolymorphicSerializer (Kotlin#2189)"

This reverts commit 3555872.

* Revert "Fix incorrect json decoding iterator's .hasNext() behavior on array-wrapped inputs: (Kotlin#2268)"

This reverts commit 2864aea.

* Revert "Fix memory leak caused by invalid KTypeWrapper's equals method (Kotlin#2274)"

This reverts commit 836f2bd.

* Revert "Replace deprecated ThreadLocal with kotlin.native.concurrent.ThreadLocal (Kotlin#2266)"

This reverts commit d73f6e3.

* Revert "Get rid of deprecated toChar() in JS-specific code (Kotlin#2252)"

This reverts commit 3686362.

* Revert "Rename json-okio `target` variables to `sink` (Kotlin#2226)"

This reverts commit a978cf0.

* Revert "Replace `runCatching-map-getOrDefault` in caching (Kotlin#2248)"

This reverts commit da91066.

* Revert "Configure project settings for Intellij IDEA (Kotlin#2217)"

This reverts commit d519d45.

* Revert "Updated K/N targets in accordance with official recommendations (Kotlin#2216)"

This reverts commit fe63ced.
bcmedeiros added a commit to bcmedeiros/ktor that referenced this pull request May 12, 2023
bcmedeiros added a commit to bcmedeiros/ktor that referenced this pull request May 12, 2023
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