Skip to content

Conversation

@glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Sep 29, 2021

Motivation:

Interceptor pipelines hold arrays of interceptor contexts. Each context
maintains a reference back to the pipeline. However the client
interceptor pipeline never broke this reference cycle when closing the
pipeline which means that using client interceptors introduces a memory
leak.

Modifications:

  • Explicitly remove all user contexts when the client interceptor pipeline is
    torn down (the server pipeline already does this so is unaffected)
  • Add allocation tests so we can see the overhead of using interceptors
    (and, more importantly, whether allocations are freed)

Result:

Using client interceptors no longer leaks.

Motivation:

Interceptor pipelines hold arrays of interceptor contexts. Each context
maintains a reference back to the pipeline. However the client
interceptor pipeline never broke this reference cycle when closing the
pipeline which means that using client interceptors introduces a memory
leak.

Modifications:

- Explicitly remove all user contexts when the client interceptor pipeline is
  torn down (the server pipeline already does this so is unaffected)
- Add allocation tests so we can see the overhead of using interceptors
  (and, more importantly, whether allocations are freed)

Result:

Using client interceptors no longer leaks.
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Sep 29, 2021
@glbrntt glbrntt requested a review from Lukasa September 29, 2021 13:22
Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Nice patch

@glbrntt glbrntt merged commit 5ddbb61 into grpc:main Sep 29, 2021
@glbrntt glbrntt deleted the gb-interceptor-pipeline-leak branch September 29, 2021 13:41
bimawa pushed a commit to StreamLayer/grpc-swift that referenced this pull request Nov 10, 2021
Motivation:

Interceptor pipelines hold arrays of interceptor contexts. Each context
maintains a reference back to the pipeline. However the client
interceptor pipeline never broke this reference cycle when closing the
pipeline which means that using client interceptors introduces a memory
leak.

Modifications:

- Explicitly remove all user contexts when the client interceptor pipeline is
  torn down (the server pipeline already does this so is unaffected)
- Add allocation tests so we can see the overhead of using interceptors
  (and, more importantly, whether allocations are freed)

Result:

Using client interceptors no longer leaks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants