Missing message possible reasons

[samthaku]

Hi,

We are using KryoNet 2.2.1 for our client-Server communication.

We are using pluggable Serialization using Simple Binary Encoding protocol.

We have recently seen issues where Message are disappearing on the wire i.e. Message are sent from the Sender but are not Received in receiver.

Messages sent: 1-4
Client received: 1,3 and 4. 2 went missing

Both Server and Client have read/write buffer sizes of 1MB.

Just wanted to understand if any such issues have been reported before and is there a known fix /reason for these issues?

What possibly can go wrong, as we have checked our code, it looks fine.

[crykn]

Do you use TCP or UDP?

[samthaku]

I'm Using TCP.

[samthaku]

We have found the cause of this issue, The pluggable serialization we are using has some state, and this had to be made thread safe. On Server side each connection has their own write-lock but multiple connections are using the same serialization instance. This Same serialization instance is used to send Keep-Alives which are sent on Server thread. While the application messages are published on Application thread.

[mihhon]

Hi, yes, you use the same instance of Serialization in all the server's Connections. If the implementation of Serialization is not threadsafe, it fails.

Server class

private void acceptOperation (SocketChannel socketChannel) {
	Connection connection = newConnection();
	connection.initialize(**serialization**, writeBufferSize, objectBufferSize);

So, if I can give a suggestion, add please comments on Serialization interface that an implementation should be threadsafe, of even better, add something like SerializationFactory that will be called in accept

Serialization newInstance(String connectionId);

Thanks

[RobertZenz]

@samthaku Please don't forget to close the issue if it is no longer relevant.

[mihhon]

@RobertZenz : it is still relevant. We found the cause of the issue, it is described above. The remedy is to synchronize #write method in Serialization , as it is done in KryoSerialization class.
But in this case Serialization is shared among all the clients connections, and all the clients connections block in #write method on the same object.

What is the rationale behind the KryoSerialization implementation - synchronizing on the instance of #read and #write of all the clients connections ?

[RobertZenz]

@mihhon Oh sorry, I parsed it as "we wrote our won serializer which had this problem".

[crykn]

@mihhon The usage of a socket factory is a good idea! I'm currently trying this out in my own fork, do you mind taking a look at it?
@samthaku Does it happen that a keep alive-message is sent simultaneously with an application message on the same connection? Or do you only send the keep alive-messages on unused connections?

[mihhon]

@crykn it happens when keep alive-message is sent on "timed out" connections simultaneously with an application message on one of connections

the piece of code below in Server in "Server" thread executes simultaneously with an application thread calling Connection#sendTcp

	long time = System.currentTimeMillis();
	Connection[] connections = this.connections;
	for (int i = 0, n = connections.length; i < n; i++) {
		Connection connection = connections[i];
		if (connection.tcp.isTimedOut(time)) {
			if (DEBUG) debug("kryonet", connection + " timed out.");
			connection.close();
		} else {
			if (connection.tcp.needsKeepAlive(time)) connection.sendTCP(FrameworkMessage.keepAlive);
		}
		if (connection.isIdle()) connection.notifyIdle();
	}
}

[crykn]

Hmm, that means that the serialization object needs to be instanciated once per message and not once per connection, doesn't it?

[mihhon]

once per connection should be enough

…

On Tue, Aug 15, 2017 at 11:06 PM, damios ***@***.***> wrote: Hmm, that means that the serialization object needs to be instanciated once per message and not once per connection, doesn't it? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#137 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AdjxP44j_u_un808-Lru5pbk8T23v4SLks5sYbQKgaJpZM4O0SSD> .

-- Greetings mihhon

[crykn]

If once per connection is enough, you could try out my fork and report back if it works.

[NathanSweet]

What is the rationale behind the KryoSerialization implementation - synchronizing on the instance of #read and #write of all the clients connections ?

KryoSerialization state is not thread safe: input buffer, output buffer, Kryo instance. Creating a Serialization per connection is not a good solution. An alternate Serialization could be written which uses thread local storage, however this is up to the Serialization implementation and what it needs to be thread safe. I don't think KryoNet needs a change, except possibly documentation.