即使地图删除了键输入，测试保持失败

Question

I have this registry struct that has a channel to unregister clients. Basically deletes the client pointer from a map and closes the client send channel. Why would this test keep failing

func (r *SocketRegistry) run() {
    for {
        select {
        case client := <-r.Register:
            r.clients[client.id] = client
        case client := <-r.UnRegister:
            delete(r.clients, client.id)
            close(client.send)
        case payload := <-r.Broadcast:
            var regPayload RegistryPayload
            json.Unmarshal(payload, &regPayload)
            client := r.clients[regPayload.ClientID]
            select {
            case client.send <- payload:
            default:
                close(client.send)
                delete(r.clients, client.id)
            }
        }
    }
}

//GetClient returns the Client pointer by id

func (r *SocketRegistry) GetClient(id string) (*Client, bool) {
    if client, ok := r.clients[id]; ok {
        return client, ok
    }
    return &Client{}, false
}

Here is the Test

func TestRegisterClient(t *testing.T) {
    registry := Registry()
    go registry.run()
    defer registry.stop()
    var client Client
    client.id = "PROPS"
    client.send = make(chan []byte, 256)

    registry.Register <- &client

    c, _ := registry.GetClient(client.id)
    if client.id != c.id {
        t.Errorf("Expected client with id: %v got: %v", client.id, c.id)
    }

    registry.UnRegister <- &client
    c, ok := registry.GetClient(client.id)
    if ok {
        t.Errorf("Expected false got ok: %v and client id: %v got: %v", ok, client.id, c.id)
    }

}

it's as if the map never deletes the key. If I add some log statements then it does delete the key, which makes me think that maybe this is a timing issue with goroutines

Answer 1

There's a race. There's no guarantee that run() executes delete(r.clients, client.id) before registry.GetClient(client.id) is called.

The race detector detects and reports the issue.

Implement GetClient like this:

// add this field to Registry
get chan getRequest

struct getRequest struct {
     ch chan *Client
     id string
}

func (r *SocketRegistry) GetClient(id string) (*Client, bool) {
    ch := make(chan *Client)
    r.get <- getRequest{id, ch}
    c := <- ch
    if c == nil {
       return &Client{}, false
    }
    return c, true
}

func (r *SocketRegistry) run() {
    for {
        select {
        case gr := <-r.get:
          gr.ch <- r.clients[id]
        case client := <-r.Register:
          ... as before
    }
}

I'd use a mutex instead of a channels and goroutine to solve this problem:

func (r *SocketRegistry) register(c *Client) {
    r.mu.Lock()
    defer r.mu.Unlock()
    r.clients[c.id] = c
}

func (r *SocketRegistry) unregister(c *Client) {
    r.mu.Lock()
    defer r.mu.Unlock()
    delete(r.clients, c.id)
    close(c.send)
}

func (r *SocketRegister) get(id string) (*Client, bool) {
    r.mu.Lock()
    defer r.mu.Unlock()
    c, ok := r.clients[id]
    return c, ok
}

func (r *SocketRegistry) send(id string, data []byte) {
   r.mu.Lock()
   defer r.mu.Unlock()
   c := r.clients[id]
   select {
   case c.send <- data:
   default:
      close(c.send)
      delete(r.clients, c.id)
   }
}

Goroutines are awesome, but they are not always the best tool for a given job.