datarhei / gosrt

Implementation of the SRT protocol in pure Go
https://datarhei.com
MIT License
114 stars 17 forks source link

split ListenAndServe() into Listen() and Serve() and fix race condition in tests #24

Closed aler9 closed 1 year ago

aler9 commented 1 year ago

Hello again,

This code won't produce the desired effect:

    wgListen := sync.WaitGroup{}
    wgListen.Add(1)
    defer server.Shutdown()

    go func() {
        wgListen.Done()
        if err := server.ListenAndServe(); err != nil {
            if err == ErrServerClosed {
                return
            }

            if !assert.NoError(t, err) {
                panic(err.Error())
            }
        }
        require.NoError(t, err)
    }()

    // Wait for goroutine to be started
    wgListen.Wait()

It indeed waits for the routine to start, but it doesn't wait for the server to start. This is because it's currently impossible to split the listening from the serving operation.

This patch splits the Server.ListenAndServe() method into two submethods, Listen() and Serve(), in a way very similar to what the net/http library does:

https://cs.opensource.google/go/go/+/refs/tags/go1.20.6:src/net/http/server.go;l=3270

In this way it's possible to build software that can check listening errors before starting a new routine, and it's also possible to fix tests - the code written above becomes:

    err := server.Listen()
    require.NoError(t, err)

    defer server.Shutdown()

    go func() {
        err := server.Serve()
        if err == ErrServerClosed {
            return
        }
        require.NoError(t, err)
    }()

Now there's a 100% certainty that the listener is available on the main routine.