XMPPConnection constructor issues

Steve_Reichert · September 8, 2004, 12:39pm

We have a couple of issues concerning the behavior of the XMPPConnection constructor (which also affect the SSLXMPPConnection constructor). The problem we are dealing with is that on several of our systems it takes a long time to establish the socket connection back to the XMPP server, and establishing the connection is not generally successful. In trying to establish the connection, the following exception is thrown (Smack 1.3.0):

Connection failed. No response from server.:

at org.jivesoftware.smack.PacketReader.startup(PacketReader.java:192)

at org.jivesoftware.smack.XMPPConnection.init(XMPPConnection.java:737)

at org.jivesoftware.smack.SSLXMPPConnection.(SSLXMPPConnection.java:120)

The first issue that we have encountered is that in the PacketReader.startup() method, the timeout is hard-wired to 5000 ms. This appears to be the case in both 1.3.0 and 1.4.0.

The second issue that we have encountered is that the XMPPConnection.init() method, which is called by the constructor, constructs both the PacketReader and PacketWriter objects which in turn start 2 threads (one for each) running. There is no issue with constructing the objects and starting the threads unless the exception noted above is thrown. In that case, both of the threads are left hanging around since the shutdown() methods are not called in the event of an exception.

These issues are amplified when our application employs its retry logic in the event of a connection failure. For each retry that occurs, the threads are left hanging around. In the case of a memory constrained JVM, this will eventually cause an out of memory condition in the JVM.

Is there a way to do the following?

Make the startup timeout configurable, as in possibly using the packet reply timeout or a new timeout value. I noticed a post from Nov 3, 2003 (Re: Login Timouts - setTimeout(int millisec)) that indicated this might be dealt with in the next release, but I didn’'t see anything in 1.4.0 regarding it.
In the event of a connection failure, cleaning up any resources that were allocated - shutting down the reader/writer threads, etc.

Perhaps I’'ve misinterpreted the behavior of the constructor, etc. If so, let me know.

Thanks,

Steve

Gaston_Dombiak · September 8, 2004, 12:52pm

Steve,

Good point! I’'ll file a bug issue for this.

Is there a way to do the following?

Make the startup timeout configurable, as in

possibly using the packet reply timeout or a new

timeout value. I noticed a post from Nov 3, 2003 (Re:

Login Timouts - setTimeout(int millisec)) that

indicated this might be dealt with in the next

release, but I didn’'t see anything in 1.4.0 regarding

it.

For now you can try replacing the hardcoded value with the one provided by SmackConfiguration. FYI, the post of Nov 3, 2003 was accomplished by implementing SmackConfiguration.getPacketReplyTimeout.

Thanks,

– Gato

Steve_Reichert · September 8, 2004, 12:59pm

Thanks for the quick reply.

Any thoughts on cleaning up the reader/writer threads?

Thanks,

Steve

Gaston_Dombiak · September 8, 2004, 1:09pm

Steve,

Any thoughts on cleaning up the reader/writer

threads?

Not yet. But I guess that modifying the hardcoded value should be enough to avoid the whole problem.

Regards,

– Gato

Steve_Reichert · September 8, 2004, 1:20pm

Yes, setting the timeout will definitely help. It would be nice though if the resources were cleaned up just in case.

Thanks,

Steve

Gaston_Dombiak · September 8, 2004, 1:27pm

Steve,

Definitely. My comment was intended as a quick workaround that solves both issues. Nevertheless, we need to solve both issues.

Thanks,

– Gato

Steve_Reichert · September 9, 2004, 11:30am

Thanks again for the reply.

Do you anticipate incorporating the changes into a 1.4.1 release or holding off until you get to a 1.5.0 release? Just trying to get an idea of when I should be looking for them.

Thanks,

Steve

Gaston_Dombiak · September 9, 2004, 6:07pm

For now the idea is to go directly to 1.5.0. However, if we find important problems with this version we could try to release an intermediate version (i.e. 1.4.1).

Regards,

– Gato

Steve_Reichert · September 15, 2004, 11:50pm

Gato,

In working to resolve the issues relating to the 5000 ms. timeout in PacketReader and the dangling threads, I’'ve made some modifications to the Smack 1.4.0 sources. If you would like to see them, let me know how to get them to you. The changes can be summarized as follows:

Modified SmackConfiguration.java and added “setConnectionTimeout()” and “getConnectionTimeout()” methods.
Modified PacketReader.java to use the timeout supplied by SmackConfiguration.getConnectionTimeout().
Modified XMPPConnection.init() to include a try/catch block that cleans up all of the reader/writer resources and closes the socket in the event of a failure.

The changes have made a significant improvement, first in the area of the initial connection, but especially in the area of connection retries in the event that a connection is dropped.

I had noticed before that when I encountered one of our Java applications that had a significant number of connection retries that a lot of dangling threads appeared and memory usage jumped. The try/catch block in XMPPConnection has helpe this significantly.

Also, were you aware that the keep alive thread never gets shut down, not even when you close the connection? For our purposes I pass a timeout value of -1 which turns it off and never starts the thread.

I’‘m curious as to whether or not I’‘m on the right track with these suggested changes or if I’'m completely off base.

Thanks,

Steve

Gaston_Dombiak · September 16, 2004, 1:51pm

Steve,

Cool!!! You can send the changes to my hotmail account and I’'ll review and incorporate them for the next release.

I have a comment on the first two changes. I think that we could use the existing message #getPacketReplyTimeout in SmackConfiguration instead of creating #get/setConnectionTimeout. The reason for this is that the connection with the server has already been made and the PacketReader is actually waiting for a response from the server to the sent “stream:stream” packet. What do you think?

Thanks for the help,

– Gato

Steve_Reichert · September 16, 2004, 4:40pm

I’'ve sent the updates as I have them right now to your hotmail account. Let me know when you get them.

You’‘ll also notice I’'ve included an “ant.sh” - I do most of my builds, etc. under Linux.

I could argue either way with SmackConfiguration. If you’‘re more comfortable with using getPacketReplyTimeout() that’‘s fine. If you prefer to go this way, let me know and I’‘ll change the copy I’'m working with and resend you the updates.

Thanks,

Steve

Gaston_Dombiak · September 21, 2004, 12:49pm

Steve,

I’‘m sorry…but it seems that hotmail classified you email as junk and discarded it. I changed the account’'s configuration. So could you send me your email again?

BTW, could you change your code to use the existing #getPacketReplyTimeout() message? I prefer to use this message for the reasons I stated before.

Thanks,

– Gato

Steve_Reichert · September 21, 2004, 1:54pm

Gato,

Yes, I will alter the code to use the existing getPacketReplyTimeout() and resend it to you. It will be a bit later today.

Steve

Steve_Reichert · September 21, 2004, 6:59pm

Gato,

I’'ve sent you the changes.

Steve

Steve_Reichert · September 21, 2004, 7:03pm

Gato,

I just got back the following response from trying to send you the updates (the zip file is 18KB):

This message is larger than the current system limit or the recipient’'s mailbox is full. Create a shorter message body or remove attachments and try sending it again.

Do you have a public FTP site or something that I can send them to?

Steve

Gaston_Dombiak · September 21, 2004, 7:06pm

hotmail…hotmail…hotmail!!!

Try with dombiak_gaston at yahoo dot com.

Thanks,

– Gato