Connection#addPacketSendingListener Javadoc is confusing/misleading

Smack Smack Dev
1

The Javadoc states that:

“Note that the thread that writes packets will be used to invoke the listeners.”

If you know the Smack internals you will know that there exists a dedicated writer thread.

If you just look at the PacketWriter class you will discover that the “sending/writer” thread

is the thread that just called Connection#sendPacket(…) and so it will be responsible for the

invocation of the listener callbacks.

It is just a little bit confusing

Furthermoore the Javadoc reads:

“The listener will be notified of every packet that this connection sends.”

This is not true. The listener will be notified about every packet that the connection is about to send, because

calling XMPPConnection#sendPacket(…) will just add the packet to the internal buffer. Afterwards it is still possible

the the connection will break and so this packet will never hits the wire at all.

BR

2

Well of course if you don’t know that there is a PacketWriter thread you may wonder which thread is meant. How do you suggest we could improve that?

“The listener will be notified of every packet that this connection sends.” →

“The listener will be notified of every packet that this connection tries to send.”

But then again, with XEP198, we may change the sematic that only ack’d stanzas are processed by the listener. Will keep that in mind.

3

I think you got me wrong here. What is confusing is the fact that if you are aware of the PacketWriter you might think that this thread will call the listeners, which is wrong.

The next problem applies to developers who are not aware of that additional PacketWriter thread. The method is called #addPacketSendingListener but the Javadoc talks about “writer thread”. The developer may

has no clue about what this Javadoc is trying to explain.

Personally I think you just can adapt the Javadoc of the #addPacketWriterInterceptor method with a remark that the listener will go to see a possible changed Packet (modified through the Interceptors).

Btw, the Javadoc should state that #addPacketSendingListener listeners should not try to mutate those packets if the Packet#toXML method is not threadsafe. (even if the method is threadsafe they must not mutate them at all as they would just act as another Interceptor with a very bad concurrency experience )

As you already mentioned that all this will be changed anyways with XEP-198 you should just keep this thread topic as a reminder to avoid the problems again

BR