Skip to content
This repository has been archived by the owner on May 26, 2022. It is now read-only.

implement a Transport.Close that waits for the reuse's GC to finish #211

Merged
merged 4 commits into from
Jul 20, 2021

Conversation

marten-seemann
Copy link
Collaborator

This finally makes it possible to run tests with race detector, and therefore switch to the Unified CI Setup.

Not sure if the implementation of Transport.Close here is sufficient to fix #176 though.

reuse.go Outdated
defer r.mutex.Unlock()
close(r.closeChan)
if r.garbageCollectorRunning {
<-r.garbageCollectorStopChan
Copy link
Member

Choose a reason for hiding this comment

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

This can deadlock if we're waiting for the garbage collection routine to take the lock.

@marten-seemann
Copy link
Collaborator Author

I removed the rather complicated logic of starting and stopping the reuse gc function. Now that we have a clean shutdown procedure, this won't be necessary any more.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

I removed the rather complicated logic of starting and stopping the reuse gc function. Now that we have a clean shutdown procedure, this won't be necessary any more.

We're still stopping the garbage collector when we have no more connections. We need to remove that if we're going to remove the auto start/stop feature.

@marten-seemann
Copy link
Collaborator Author

We're still stopping the garbage collector when we have no more connections. We need to remove that if we're going to remove the auto start/stop feature.

Oops, must have missed that. Fixed.

@Stebalien
Copy link
Member

LGTM but there's a test failure we should probably investigate.

@marten-seemann
Copy link
Collaborator Author

LGTM but there's a test failure we should probably investigate.

Looks unrelated, I opened #213.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

implement transport.Close()
2 participants