containerd / fifo

fifo pkg for Go
https://containerd.io
Apache License 2.0
85 stars 30 forks source link

[Bug]:A potential goroutine leak #56

Open xuxiaofan1203 opened 5 months ago

xuxiaofan1203 commented 5 months ago

Hello @samuelkarp, When I used fifo, I found a potential bug, I'm not sure, maybe we can discuss to avoid a goleak blocking position: https://github.com/containerd/fifo/blob/3e17f98903a83f1a6c7368ea4694e6861161d6bd/fifo.go#L124-L133 if users use the OpenFilo() with the parameters like this: f, err := OpenFifo(context.Background(), filepath.Join(tmpdir, "f0"), syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0600) The select statement will block beacuse there is no cancelFunc to awaken the <-ctx.Done().

I wrote a test function to reproduce the bug considering the test that https://github.com/containerd/fifo/blob/3e17f98903a83f1a6c7368ea4694e6861161d6bd/fifo_test.go#L47

func TestFifoNocancel(t *testing.T) {
    defer goleak.VerifyNone(t)
    tmpdir, err := os.MkdirTemp("", "fifos")
    assert.NoError(t, err)
    defer os.RemoveAll(tmpdir)

    leakCheckWg = &sync.WaitGroup{}
    defer func() {
        leakCheckWg = nil
    }()

    //f, err := OpenFifo(context.Background(), filepath.Join(tmpdir, "f0"), syscall.O_RDONLY|syscall.O_NONBLOCK, 0600)
    f, err := OpenFifo(context.Background(), filepath.Join(tmpdir, "f0"), syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0600)
    assert.Exactly(t, nil, f)
    assert.NotNil(t, err)
    assert.NoError(t, checkWgDone(leakCheckWg))
}

The test result shows it is a goroutine leak, you can use goleak to reproduce the bug. 1712307934228