[Bug]: Read returns io.ErrShortBuffer on closed connection

[ezh]

Actions I've taken before I'm here

• [X] I've thoroughly read the documentations on this issue but still have no clue.
• [X] I've searched the Github Issues but didn't find any duplicate issues that have been resolved.
• [X] I've searched the internet for this issue but didn't find anything helpful.

What happened?

https://github.com/panjf2000/gnet/blob/master/connection_unix.go#L306

func (c *conn) Read(p []byte) (n int, err error) {
    if c.inboundBuffer.IsEmpty() {
        n = copy(p, c.buffer)
        c.buffer = c.buffer[n:]
->      if n == 0 && len(p) > 0 {
            err = io.ErrShortBuffer
        }
        return
    }
    n, _ = c.inboundBuffer.Read(p)
    if n == len(p) {
        return
    }
    m := copy(p[n:], c.buffer)
    n += m
    c.buffer = c.buffer[m:]
    return
}

A closed connection with an empty buffer(because of a lack of incoming data) returns a ShortBuffer error

NB. opened connection also returns a shortbuffer when there is no incoming data, but it is not so critical 🙈 .

Major version of gnet

v2

Specific version of gnet

v2.5.1

Operating system

macOS

OS version

Darwin 23.4.0 arm64

Go version

go version go1.22.2 darwin/arm64

Relevant log output

The observability is far from the best since it is the simplest case.

[gnet] 2024-04-28T15:52:50.015965+04:00 INFO    logging/logger.go:256   Starting gnet client with 1 event-loop
[gnet] 2024-04-28T15:52:50.016327+04:00 DEBUG   logging/logger.go:249   default logging level is DEBUG
[gnet] 2024-04-28T15:52:56.135536+04:00 DEBUG   v2/reactor_default.go:105       event-loop(0) is exiting in terms of the demand from user, gnet: server is going to be shutdown

Code snippets (optional)

No response

How to Reproduce

create listener like

    conn, err := l.Listener.Accept()
    if err != nil {
        return nil, err
    }
    gConn, err := client.EnrollContext(conn, l)
    if err != nil {
        return nil, l.GetError()
    }

And connect with the TCP session.

clientConn, err := net.Dial("tcp", l.Addr().String())
Expect(err).NotTo(HaveOccurred())

Than, on an empty connection, do

n, err := conn.Read(buf[totalRead:])

it will return io.ErrShortBuffer

close connection in a go routine

do conn.Read on more time on closed connection and 🤷‍♂️

Does this issue reproduce with the latest release?

It can reproduce with the latest release

[ezh]

I can apply a workaround, but certainly, preemptively checking the connection status before each read would not be the most efficient or elegant solution.

[ezh]

Honestly, I'm quite surprised that huge corporations are using the code with such behavior in production. Thank you for the code, anyway.

[panjf2000]

I fail to see what is bothering you. What's your problem with io.ErrShortBuffer?

[ezh]

io.ErrShortBuffer indicates at the same time:

1. there is a short buffer
2. there is not enough data
3. there is a closed connection For example, if there is not enough data, the read operation should be repeated after a while to get a new portion. However, there is no reason to do that in case of a closed connection. Am I wrong? I may have missed an example of how to do it correctly.

We expect to get 3 bytes.

1. conn.Read returns io.ErrShortBuffer because there is no data at all
2. do conn.Read one more time, the client sends 1 byte, and we have io.ErrShortBuffer again
3. the connection took too long, we close the connection in a separate goroutine
4. conn.Read returns io.ErrShortBuffer again

Do you think the behavior is correct? If so, I'll close the report. 🤷‍♂️

[ezh]

I understand that we can track the status with the event handler, or even write the custom state machine.

But my question is about conn.Read behavior.

[panjf2000]

gnet is a event-driven framework, you should only read data in OnTraffic, why would you call Read() constantly to check if there is data coming? I don't think you're using gnet in the right way.

[ezh]

Thank you for your quick feedback. I appreciate your guidance and will verify the usage to align with best practices.

Maybe I didn't get the idea.