Occasional 'slice bounds out of range' panic

Issue

I’m running a script that forwards webhooks to a WebSocket.
The part that sends the webhook to the WebSocket checks for inactive connections and tries to remove them when forwarding webhooks sometimes fails with this error:

http: panic serving 10.244.38.169:40958: runtime error: slice bounds out of range

(The IP/port is always different, this is just an example.)

Relevant code:

// Map holding all Websocket clients and the endpoints they are subscribed to
var clients = make(map[string][]*websocket.Conn)
var upgrader = websocket.Upgrader{}

// function to execute when a new client connects to the websocket
func handleClient(w http.ResponseWriter, r *http.Request, endpoint string) {
    conn, err := upgrader.Upgrade(w, r, nil)
    // ...
    // Add client to endpoint slice
    clients[endpoint] = append(clients[endpoint], conn)
}

// function to send a webhook to a websocket endpoint
func handleHook(w http.ResponseWriter, r *http.Request, endpoint string) {
    msg := Message{}
    // ...   
    // Get all clients listening to the current endpoint
    conns := clients[endpoint]

    if conns != nil {
        for i, conn := range conns {
            if conn.WriteJSON(msg) != nil {
                // Remove client and close connection if sending failed
                conns = append(conns[:i], conns[i+1:]...)   // this is the line that sometimes triggers the panic
                conn.Close()
            }
        }
    }

    clients[endpoint] = conns
}

I cannot figure out why iterating over the connections and appending them sometimes triggers the panic.

Solution

Few points that I’d like to say:

  1. Make sure that your program has no race condition (eg. clients is globally accessible and should be protected if read/write or write/write
    happening concurrently).

  2. When ranging over a slice for [...] range [...] you do not need to check if slice the non-nil as range handles that already (see the code I’ve shared).

  3. It is happening to your sometimes because there are times when conn.WriteJSON is failing and returning an error and the buggy logic of deleting element while ranging over is making your program panic. (see the code I’ve shared)

package main

import "fmt"

func main() {
    var conns []string = nil

    // "if conns != nil" check is not required as "for [...] range [...]"
    // can handle that. It is safe to use for "range" directly.
    for i, conn := range conns {
        fmt.Println(i, conn)
    }

    conns = []string{"1", "2", "3"}
    
    // Will panic
    for i := range conns {
        fmt.Printf("access: %d, length: %d\n", i, len(conns))
        conns = append(conns[:i], conns[i+1:]...)
    }
}

In the example, you can see the index that you are trying to access it more than or equal to the length of the slice which is triggering the panic. I think this answer should help you to correct your logic or you can use a map as well for storing connections but it again comes with its own caveats like no ordering guarantee i.e., in which order it reads from the map.

Answered By – shmsr

Answer Checked By – Willingham (GoLangFix Volunteer)

Leave a Reply

Your email address will not be published.