Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Questions about Socket's sendPacket and flush implementation #119

Open
Piasy opened this issue Dec 29, 2024 · 0 comments
Open

Questions about Socket's sendPacket and flush implementation #119

Piasy opened this issue Dec 29, 2024 · 0 comments

Comments

@Piasy
Copy link

Piasy commented Dec 29, 2024

Hi, I've been working on porting Java implementation of EngineIO and SocketIO to Kotlin Multiplatform recently.

During the development and test process, I have a question about Socket's sendPacket and flush implementation:

https://github.com/socketio/engine.io-client-java/blob/main/src/main/java/io/socket/engineio/client/Socket.java#L696C1-L712C6

    private void sendPacket(Packet packet, final Runnable fn) {
        if (ReadyState.CLOSING == this.readyState || ReadyState.CLOSED == this.readyState) {
            return;
        }

        this.emit(EVENT_PACKET_CREATE, packet);
        this.writeBuffer.offer(packet);
        if (null != fn) {
            this.once(EVENT_FLUSH, new Listener() {
                @Override
                public void call(Object... args) {
                    fn.run();
                }
            });
        }
        this.flush();
    }

https://github.com/socketio/engine.io-client-java/blob/main/src/main/java/io/socket/engineio/client/Socket.java#L617C1-L627C6

    private void flush() {
        if (this.readyState != ReadyState.CLOSED && this.transport.writable &&
                !this.upgrading && this.writeBuffer.size() != 0) {
            if (logger.isLoggable(Level.FINE)) {
                logger.fine(String.format("flushing %d packets in socket", this.writeBuffer.size()));
            }
            this.prevBufferLen = this.writeBuffer.size();
            this.transport.send(this.writeBuffer.toArray(new Packet[this.writeBuffer.size()]));
            this.emit(EVENT_FLUSH);
        }
    }

When we want to send a packet, we add it to writeBuffer and trigger flush, and we send all packets in writeBuffer to transport, and we remove this amount of packets from writeBuffer in onDrain.

But if we call send multiple times before onDrain is triggered, let's say 3 times, with pkt1, pkt2, pkt3, then we will call transport.send 3 times, with [pkt1], [pkt1, pkt2], and [pkt1, pkt2, pkt3], and that's definitely a wrong behavior.

Do I miss something to prevent this case from happening?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant