search

lorenzodonini / ocpp-go

Open Charge Point Protocol implementation in Go
MIT License
275 stars 126 forks source link

Don't prevent ocppj goroutine from closing #75

Closed michaelbeaumont closed 3 years ago

michaelbeaumont commented 3 years ago

I was just playing around with go.uber.org/goleak and saw messagePump hanging around quite often. If we immediately set d.requestChannel to nil and messagePump isn't already waiting to select from d.requestChannel, it will never return. messagePump already sets the channel to nil as well.

michaelbeaumont commented 3 years ago

I've "fixed" the tests with a wait but the right way to do it is to have Start() (or IsRunning() at least) return a chan struct{} that can be checked to see if the server is running and then close this channel when messagePump exits, but that requires interface changes.

michaelbeaumont commented 3 years ago

@lorenzodonini Are you happy with IsRunning not "immediately" returning false for now?

lorenzodonini commented 3 years ago

@lorenzodonini Are you happy with IsRunning not "immediately" returning false for now?

I think it's fine for now.

I'm honestly skeptical to adding a channel for this purpose though: client.go doesn't have an event loop, so it would be impractical to check whether that new channel was closed in any way.

I'll think about a possible alternative. Merging this for now 👍