Skip to content

Conversation

AArnott
Copy link
Member

@AArnott AArnott commented Aug 13, 2024

The collection of joinable tasks for which an ID is requested grew forever. The code that should purge completed tasks from the collection was never executing because the TaskId property is programmed to return null for completed tasks, and we query the property directly after setting the task as completed.

The fix then is simple: fetch the TaskId value first and then set the JoinableTask as completed.

@sharwell found this and reports that 361MB was held in memory due to this leak in one dump he investigated.

This fixes a memory leak that impacts versions as far back as 17.6. This PR only applies the fix to 17.12, but the commit itself is based on the original bug so the fix can be merged anywhere necessary.

The collection of joinable tasks for which an ID is requested grew forever. The code that should purge completed tasks from the collection was never executing because the `TaskId` property is programmed to return `null` for completed tasks, and we query the property directly *after* setting the task as completed.

The fix then is simple: fetch the TaskId value *first* and *then* set the `JoinableTask` as completed.
@AArnott AArnott added this to the v17.12 milestone Aug 13, 2024
Copy link
Member

@BertanAygun BertanAygun left a comment

Choose a reason for hiding this comment

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

Nice find!

@AArnott AArnott merged commit 243b960 into microsoft:main Aug 13, 2024
@AArnott AArnott deleted the fixserializedTaskLeak branch August 13, 2024 22:13
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.

2 participants