vert-x3 / vertx-stomp

STOMP client/server implementation
Apache License 2.0
30 stars 27 forks source link

STOMP server tasks(pinger/ponger) runs endlessly #61

Open anatoliyck opened 4 years ago

anatoliyck commented 4 years ago

Hi, I have an issue with a lot of logs about idle connections on the STOMP server(3.7.1 version) with websocket bridge.

Example from my logs:

11:35:18.486 WARN Disconnecting client io.vertx.ext.stomp.impl.StompServerWebSocketConnectionImpl@6f5765e1 - no client activity in the last 329665005 ms

11:35:18.442 WARN Disconnecting client io.vertx.ext.stomp.impl.StompServerWebSocketConnectionImpl@6f5765e1 - no client activity in the last 329664961 ms

11:35:13.486 WARN Disconnecting client io.vertx.ext.stomp.impl.StompServerWebSocketConnectionImpl@6f5765e1 - no client activity in the last 329660005 ms

As you can see this hashCode is the same for all logs.

Logs count(only for this connection):

  1. last 8 hours: 11,541
  2. all time: 133,148

Clients use the following heart-beat value: 5000,10000

UPD: I reproduced this issue -> client sends more than one CONNECT command. A quick solution is a change StompServerTCPConnectionImpl#configureHeartbeat to:

public synchronized void configureHeartbeat(long ping, long pong, Handler<StompServerConnection> pingHandler) {
    cancelHeartbeat();

    if (ping > 0) {
      pinger = server.vertx().setPeriodic(ping, l -> pingHandler.handle(this));
    }
    if (pong > 0) {
      ponger = server.vertx().setPeriodic(pong, l -> {
        long delta = System.nanoTime() - lastClientActivity;
        final long deltaInMs = TimeUnit.MILLISECONDS.convert(delta, TimeUnit.NANOSECONDS);
        if (deltaInMs > pong * 2) {
          log.warn("Disconnecting client " + this + " - no client activity in the last " + deltaInMs + " ms");
          close();
        }
      });
    }
  }

But we should handle this case normally.

vietj commented 4 years ago

is that expected to have a client sending more than one CONNECT ?

vietj commented 4 years ago

I think it would be better to have instead a boolean set to true when client connected so we don't handle the connect procedure twice.

also the heartbeat code seems similar in client and server, it would be good to have this code factored in a class for using in both cases

anatoliyck commented 4 years ago

is that expected to have a client sending more than one CONNECT ?

No, its client bug, but server should handle it.

vietj commented 4 years ago

@anatoliyck agreed, was just wondering