Is there any way to send bundles of packets to the client in a thread safe way?

[jobpaardekooper]

Is your feature request related to a problem? Please describe. The protocol includes a client bound Bundle Delimiter to send packets to the client that should be processed in the same tick. The description from that wiki page discribes it as the following:

The delimiter for a bundle of packets. When received, the client should store every subsequent packet it receives, and wait until another delimiter is received. Once that happens, the client is guaranteed to process every packet in the bundle on the same tick, and the client should stop storing packets. For example, this is used to ensure Spawn Entity and Set Entity Metadata happen on the same tick.

Is there currently a way to use this feature in a thread safe way?

We can of course use this packet by just sending it using conn.WritePacket to indicate the start of the bundle. Then use conn.WritePacket to send some packets. Then we can indicate the end of the bundle with conn.WritePacket. But this is not thread safe. Of course nothing will break anything in a big way but if you have a multithreaded sever there is a chance these "Bundle Delimiters" get interleaved with ones from other threads. Which would basically defeat their purpose.

I was wondering if there is currently an API to write multiple packets at once?

[maxsupermanhd]

Unfortunately, as with everything in Tnze's go-mc library, if you want to deviate from original goals and vision of Tnze on how it should look+work you will have to essentially rewrite everything from scratch.

[jobpaardekooper]

I have also seen you arguing in some other issues? Whats the problem here?

[maxsupermanhd]

Problem is that go-mc is advertised as ultimate golang library for everything in minecraft but in reality it is just Tnze doing what he wants with 0 care about the others. Read issues themselves and discord conversations if you are interested.

[Tnze]

@jobpaardekooper Hi, which package are you talking about, the net or bot ? The net package doesn't have any thread safety guarantee provided. But the bot package uses safety queues for sending and receiving packets.

Is there currently a way to use this feature in a thread safe way?

When you are using the bot package: Yes, you can provide your own Queue implement in the JoinOptions Then you can prevent other thread pushing packets by locking an internal Mutex (when you are pushing a packet bundle).

[Tnze]

Such implement can be something better, but for the simplest way: Copy net/queue.LinkedListQueue and add methods like this:

func (p *LinkedListQueue[T]) PushBundle(v ...T) bool {
    p.cond.L.Lock()
    if p.closed {
        panic("push on closed queue")
    }
    for _, vv := range v {
        p.queue.PushBack(vv)
    }
    p.cond.Signal()
    p.cond.L.Unlock()
    return true
}

The problem is hot to call this method when you have a bot.Conn. Feel free to clone and modify this library. If the solution works, you can create a PR to merge it to go-mc.

[Tnze]

Or maybe a better option is adding a Mutex for send queue in bot.Conn? And then extend the WritePacket method:

func (c *Conn) WritePacket(packets ...pk.Packet) error {
    c.sendLock.Lock()
    defer c.sendLock.Unlock()
    for _, p := range packets {
        ok := c.send.Push(p)
        if !ok {
            return errors.New("queue is full")
        }
    }
    return nil
}

[jobpaardekooper]

Thanks for the example implementation. I was actually talking about net.Conn and the WritePacket on such a connection. Why is that not thread safe? It seems like it should be. In the end it just sends out things over the standard go net.Conn and that is thread safe.

Edit: Additionally, I don't think this would make much sense to support for the bot package because it is a client bound packet. You can't do this for server bound packets. Thats why I am asking it for the net package.

[Tnze]

Why is that not thread safe? It seems like it should be.

The thought is that a Mutex should protect process, rather than objects. So in the different logic layers we need different extent atomic operations promise, which means it may be required to add multiple Mutexs.

I want to let the user to choice write multiple threaded program or not. If there is only single goroutine is operating the Conn (i.e. receiving packets from channels), the lock is not only useless but causing performance overhead, which is why go-mc/net.Conn doesn't have an internal lock -- You could add it yourself when needed.

I don't think this would make much sense to support for the bot package because it is a client bound packet.

You are right. That's my mistake.