lorenzodonini / ocpp-go

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

Why ws.outQueue is need to check if the Channel is closed? #308

Open AndrewYEEE opened 1 month ago

AndrewYEEE commented 1 month ago

Sorry I dont understand why ws.outQueue is need to check that Channel is closed ? This will cause writePump closed but server.connections still retains the ws object (ws.connection still work).

And Why doesn't ws.pingMessage need to check if the Channel closed unexpectedly?

 func (server *Server) writePump(ws *WebSocket) {
    conn := ws.connection

    for {
        select {
        case data, ok := <-ws.outQueue:
            _ = conn.SetWriteDeadline(time.Now().Add(server.timeoutConfig.WriteWait))
            if !ok {
                // Unexpected closed queue, should never happen
                server.error(fmt.Errorf("output queue for socket %v was closed, forcefully closing", ws.id))
                // Don't invoke cleanup

                return      **//<===== will exit writePump(), but server.connections still retains the WS object  !!!**
            }
            // Send data
            err := conn.WriteMessage(websocket.TextMessage, data)
            if err != nil {
                server.error(fmt.Errorf("write failed for %s: %w", ws.ID(), err))
                // Invoking cleanup, as socket was forcefully closed
                server.cleanupConnection(ws)
                return
            }
            log.Debugf("written %d bytes to %s", len(data), ws.ID())
        case ping := <-ws.pingMessage:

                       **// why don't need check channel closed unexpectedly?**

            _ = conn.SetWriteDeadline(time.Now().Add(server.timeoutConfig.WriteWait))
            err := conn.WriteMessage(websocket.PongMessage, ping)
            if err != nil {
                server.error(fmt.Errorf("write failed for %s: %w", ws.ID(), err))
                // Invoking cleanup, as socket was forcefully closed
                server.cleanupConnection(ws)
                return
            }
            log.Debugf("pong sent to %s", ws.ID())
            ...
AndrewYEEE commented 1 month ago

I have the idea to remove ws object, but I couldn't prove whether the ws object obtained by server.connections[] is the same as the one to be removed.

func (server *Server) writePump(ws *WebSocket) {
   conn := ws.connection
   for {
    select {
          case data, ok := <-ws.outQueue:

                    if !ok {
                        // Unexpected closed queue, should never happen
                        server.error(fmt.Errorf("output queue for socket %v was closed, forcefully closing", ws.id))
                        // Don't invoke cleanup

                        //delete chargepoint ws
                        server.connMutex.Lock()
                        if _, ok := server.connections[ws.id]; ok {
                            delete(server.connections, ws.id)                   // is same?
                            log.Infof("closed connection to %s", ws.ID())
                            if server.disconnectedHandler != nil {
                                server.disconnectedHandler(ws) 
                            }
                            }
                        server.connMutex.Unlock()

                        return
                    }