Panic: ring_buffer, slice bounds out of range

gvitali commented 2 years ago

Describe the bug

Gnet lib panics quite often when receives big TCP packet:

panic: runtime error: slice bounds out of range [:302481] with capacity 262144
    panic: runtime error: slice bounds out of range [:302481] with capacity 262144

goroutine 6276 [running]:
github.com/panjf2000/gnet/v2/pkg/buffer/ring.(*Buffer).Peek(0xcc823b7a10?, 0x12772c5?)
    github.com/panjf2000/gnet/v2@v2.1.1/pkg/buffer/ring/ring_buffer.go:84 +0x24a
github.com/panjf2000/gnet/v2/pkg/buffer/elastic.(*RingBuffer).Peek(...)
    github.com/panjf2000/gnet/v2@v2.1.1/pkg/buffer/elastic/elastic_ring_buffer.go:58
github.com/panjf2000/gnet/v2/pkg/buffer/elastic.(*Buffer).Peek(0xc2f3120690?, 0x2327350?)
    github.com/panjf2000/gnet/v2@v2.1.1/pkg/buffer/elastic/elastic_ring_list_buffer.go:60 +0x65
github.com/panjf2000/gnet/v2.(*eventloop).closeConn(0xc114510090, 0xc2f6a042d0, {0x0, 0x0})
    github.com/panjf2000/gnet/v2@v2.1.1/eventloop.go:198 +0x65a
github.com/panjf2000/gnet/v2.(*eventloop).closeAllSockets(0xc114510090)
    github.com/panjf2000/gnet/v2@v2.1.1/eventloop.go:67 +0x85
github.com/panjf2000/gnet/v2.(*eventloop).run.func1()
    github.com/panjf2000/gnet/v2@v2.1.1/reactor_default_linux.go:93 +0x25
panic({0x1b8eb40, 0xcc64721b78})
    runtime/panic.go:884 +0x212
github.com/panjf2000/gnet/v2/pkg/buffer/ring.(*Buffer).Peek(0x0?, 0x20?)
    github.com/panjf2000/gnet/v2@v2.1.1/pkg/buffer/ring/ring_buffer.go:84 +0x24a
github.com/panjf2000/gnet/v2/pkg/buffer/elastic.(*RingBuffer).Peek(...)
    github.com/panjf2000/gnet/v2@v2.1.1/pkg/buffer/elastic/elastic_ring_buffer.go:58
github.com/panjf2000/gnet/v2/pkg/buffer/elastic.(*Buffer).Peek(0xc2f3120690?, 0x5f5678?)
    github.com/panjf2000/gnet/v2@v2.1.1/pkg/buffer/elastic/elastic_ring_list_buffer.go:60 +0x65
github.com/panjf2000/gnet/v2.(*eventloop).write(0xc114510090, 0xc2f6a042d0)
    github.com/panjf2000/gnet/v2@v2.1.1/eventloop.go:146 +0x35
github.com/panjf2000/gnet/v2.(*eventloop).run.func2(0x725c7d6f82002108?, 0x4)
    github.com/panjf2000/gnet/v2@v2.1.1/reactor_default_linux.go:112 +0x89
github.com/panjf2000/gnet/v2/internal/netpoll.(*Poller).Polling(0xc00072a050, 0xcc823b7f88)
    github.com/panjf2000/gnet/v2@v2.1.1/internal/netpoll/epoll_default_poller.go:138 +0x2ba
github.com/panjf2000/gnet/v2.(*eventloop).run(0xc114510090, 0x0?)
    github.com/panjf2000/gnet/v2@v2.1.1/reactor_default_linux.go:98 +0xcb
github.com/panjf2000/gnet/v2.(*engine).startEventLoops.func1.1()
    github.com/panjf2000/gnet/v2@v2.1.1/engine.go:69 +0x2e
created by github.com/panjf2000/gnet/v2.(*engine).startEventLoops.func1
    github.com/panjf2000/gnet/v2@v2.1.1/engine.go:68 +0x8d

To Reproduce

It happens not always, let's say once per 1000 requests.

Code snippet:

func (ts *tcpServer) OnTraffic(c gnet.Conn) gnet.Action {
    var bytes []byte
    fromIp := c.RemoteAddr().String()
    bytesIn, err := c.Next(-1)
    if err != nil {
        log.Println(err)
    }
    tcpMessages[fromIp] += string(bytesIn)
    if !strings.Contains(tcpMessages[fromIp], "\n") {
        // message's packet is still collecting
        return gnet.None
    } else {
        // convert string message back to bytes
        bytes = []byte(tcpMessages[fromIp])
        // whole packet received
        tcpMessages[fromIp] = ""
    }
...

Expected behavior I expect to have no panic in 100% cases.

System Info (please fill out the following information):

OS: Ubuntu 20.04.5 LTS
Go version: go1.19.3 linux/amd64
gnet version: 2.1.1

panjf2000 commented 2 years ago

How big is your packet?

panjf2000 commented 2 years ago

Besides, have you ever called c.ReadFrom()?

gvitali commented 2 years ago

How big is your packet?

Packets are 60 000 — 2 000 000 bytes.

Besides, have you ever called c.ReadFrom()?

Nope, it's not used

panjf2000 commented 2 years ago

Could you share more code about how you write data back to client in OnTraffic?

gvitali commented 2 years ago

Could you share more code about how you write data back to client in OnTraffic?

func sendTCPBytes(bytes []byte) {
    var err error
    var bytesWritten int

    if ConnTCP != nil && ConnTCP.RemoteAddr() != nil {
        err = ConnTCP.SetWriteBuffer(len(bytes))
        if err != nil {
            log.Println(err)
        }

        bytesWritten, err = ConnTCP.Write(bytes)
        if err != nil || bytesWritten < 1 {
            log.Println(err)
        }
    }
}

panjf2000 commented 2 years ago

Did you call sendTCPBytes() in OnTraffic or in a new goroutine?

gvitali commented 2 years ago

Did you call sendTCPBytes() in OnTraffic or in a new goroutine?

Every OnTraffic calls two sendTCPBytes(), the first one in a new goroutine and the second one without a goroutine (~1 ms later)

panjf2000 commented 2 years ago

Did you call sendTCPBytes() in OnTraffic or in a new goroutine?

Every OnTraffic calls two sendTCPBytes(), the first one in a new goroutine and the second one without a goroutine (~1 ms later)

c.Write() can only be called in the goroutine where OnTraffic resides because this method is not thread-safe (or we should say goroutine-safe), if you must write data back to the client outside of OnTraffic, use c.AsyncWrite() instead.

gvitali commented 2 years ago

c.Write() can only be called in the goroutine where OnTraffic resides because this method is not thread-safe (or we should say goroutine-safe), if you must write data back to the client outside of OnTraffic, use c.AsyncWrite() instead.

Thank you, I will try and monitor if it fixes the panic. Does c.AsyncWrite() affect to performance?

panjf2000 commented 2 years ago

c.Write() can only be called in the goroutine where OnTraffic resides because this method is not thread-safe (or we should say goroutine-safe), if you must write data back to the client outside of OnTraffic, use c.AsyncWrite() instead.

Thank you, I will try and monitor if it fixes the panic. Does c.AsyncWrite() affect to performance?

Not from my point of view.

gvitali commented 2 years ago

It seems c.AsyncWrite() has fixed the issue. But it looks dramatically (hundreds of times) slower than a writing data with c.Write(). For example, it takes about 40 ms to write data 100kb with c.AsyncWrite() and 0.1 ms using c.Write(). The time measured between sending a data and a receiving a response / callback. The difference is huge, is there anything that can be done about it?

And one more question, how can I find out the number of bytes written with c.AsyncWrite()?

panjf2000 commented 2 years ago

It seems c.AsyncWrite() has fixed the issue. But it looks dramatically (hundreds of times) slower than a writing data with c.Write(). For example, it takes about 40 ms to write data 100kb with c.AsyncWrite() and 0.1 ms using c.Write(). The time measured between sending a data and a receiving a response / callback. The difference is huge, is there anything that can be done about it?

And one more question, how can I find out the number of bytes written with c.AsyncWrite()?

Now that this turns out not a bug, I'm going to close this issue, as for the performance of AsyncWrite(), it's better to move the thread to a new issue #423 , if you want to help diagnose it, you could provide some demo code and benchmarks, thanks~

panjf2000 / gnet

Panic: ring_buffer, slice bounds out of range #422