search

lorenzodonini / ocpp-go

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

Set ws.Client disconnected on Stop #62

Closed michaelbeaumont closed 3 years ago

michaelbeaumont commented 3 years ago

Prevents the following panic:

panic: send on closed channel

goroutine 16 [running]:
github.com/lorenzodonini/ocpp-go/ws.(*Client).Write(0xc00009a790, 0xc000036720, 0x25, 0x30, 0x0, 0x0)
        /home/mike/projects/project/client/vendor/github.com/lorenzodonini/ocpp-go/ws/websocket.go:758 +0x70
github.com/lorenzodonini/ocpp-go/ocppj.(*Client).SendResponse(0xc00009e310, 0xc00003aca0, 0x9, 0xb44860, 0xc000073770, 0x7f64d4b2e108, 0x10)
        /home/mike/projects/project/client/vendor/github.com/lorenzodonini/ocpp-go/ocppj/client.go:154 +0x114
github.com/lorenzodonini/ocpp-go/ocpp1%2e6.(*chargePoint).sendResponse(0xc0000a0400, 0xb44860, 0xc000073770, 0x0, 0x0, 0xc00003aca0, 0x9)
        /home/mike/projects/project/client/vendor/github.com/lorenzodonini/ocpp-go/ocpp1.6/charge_point.go:263 +0x9a
github.com/lorenzodonini/ocpp-go/ocpp1%2e6.(*chargePoint).handleIncomingRequest(0xc0000a0400, 0xb44880, 0xc00004c900, 0xc00003aca0, 0x9, 0xc0000384f8, 0x13)
        /home/mike/projects/project/client/vendor/github.com/lorenzodonini/ocpp-go/ocpp1.6/charge_point.go:387 +0x1c5
github.com/lorenzodonini/ocpp-go/ocppj.(*Client).ocppMessageHandler(0xc00009e310, 0xc0000cb400, 0xcb, 0x200, 0x0, 0x1)
        /home/mike/projects/project/client/vendor/github.com/lorenzodonini/ocpp-go/ocppj/client.go:194 +0x29a
github.com/lorenzodonini/ocpp-go/ws.(*Client).readPump(0xc00009a790)
        /home/mike/projects/project/client/vendor/github.com/lorenzodonini/ocpp-go/ws/websocket.go:711 +0x16f
created by github.com/lorenzodonini/ocpp-go/ws.(*Client).Start
        /home/mike/projects/project/client/vendor/github.com/lorenzodonini/ocpp-go/ws/websocket.go:792 +0x5fa

which is due to handleIncomingRequest being called after we've called Stop and it calling SendResponse and then Write

lorenzodonini commented 3 years ago

This one's pretty unlucky.

Closing the outQueue channel already triggers the setConnected(false), although with scheduling in between.

You might even want to move this statement above the close statement, just to be on the safe side.

michaelbeaumont commented 3 years ago

True, updated.