FaultTolerantNegotiator Bug (Thread Leak)

Each time I receive and accept an incoming file transfer, the FaultTolerantNegotiator creates two threads using an executor service. Neither of these threads ever exits, so after several file transfers, I have a substantial number of extraneous threads running. Has anybody else seen this?

1 Like

Yes, I have observed it too. The following fix seemed to work for me:

In file FaultTolerantNegotiator.java, modified the follwing method to -

public InputStream createIncomingStream(StreamInitiation initiation) throws XMPPException {
PacketCollector collector = connection.createPacketCollector(
getInitiationPacketFilter(initiation.getFrom(), initiation.getSessionID()));

connection.sendPacket(super.createInitiationAccept(initiation, getNamespaces()));

** // ----> thread leak fix: start

    // CompletionService<InputStream> service
    //        = new ExecutorCompletionService<InputStream>(Executors.newFixedThreadPool(2));**

** ExecutorService es = Executors.newFixedThreadPool(2);
CompletionService service
= new ExecutorCompletionService(es);

    // ----> thread leak fix: end**

List<Future> futures = new ArrayList<Future>();
InputStream stream = null;
XMPPException exception = null;
try {
futures.add(service.submit(new NegotiatorService(collector)));
futures.add(service.submit(new NegotiatorService(collector)));

int i = 0;
while (stream == null && i < futures.size()) {
Future future;
try {
i++;
future = service.poll(10, TimeUnit.SECONDS);
}
catch (InterruptedException e) {
continue;
}

if (future == null) {
continue;
}

try {
stream = future.get();
}
catch (InterruptedException e) {
/* Do Nothing */
}
catch (ExecutionException e) {
exception = new XMPPException(e.getCause());
}
}
}
finally {
for (Future future : futures) {
future.cancel(true);
}
collector.cancel();

** // ----> thread leak fix: added this one line below
es.shutdownNow();
** }
if (stream == null) {
if (exception != null) {
throw exception;
}
else {
throw new XMPPException(“File transfer negotiation failed.”);
}
}

return stream;
}

Please comment on this solution if anyone notices any issues with it.

1 Like

Thanks very much for solving this. A year later and it’s still in the 3.1 code? Ah well, code was last updated long before this…

Bill

It seems this error still exists even in the SVN trunk. Could somebody please fix it.

Stefan

Entered as SMACK-353.

There is a lot of changes being made to file transfer already, so this may or may not be relevant once those changes are applied.

Hi

is there anything “visible” to the user due to this bug? We are still chasing a file transfer bug in Spark/Smack. Or is this “just” a clean up problem?

Walter

Hi Walter,

I cannot really tell at all. We (really) massively use file transfers in our project which causes many such thread-pools to be created but as far as i can tell it seems to work fine anyway.

Stefan

Hi again,

but of course it is getting “visible” as soon as the maximum amount of native threads that can be created at the same time is reached causing the JVM to crash.

Stefan

A project implies that you are involved in a Jave development project. Can you please provide a path for the 3.2. branch of Smack to get this into the 3.2. branch?

Robin Collier will make sure that this goes also in the Smack trunk.

Hi,

here it is.

Stefan
FaultTolerantNegotiator.patch.zip (1117 Bytes)

First of all Sam, thank-you. We also do large amounts of transfers using XMPP. The problem that we are looking at sounds like it is fixed by this. We’ll be testing it in our app soon.

Hi Walter,

I know everone has got a lot to do but this is a pretty serious error which could be avoided easily. Will the solution go into the trunk soon or shall I build my own version?

Stefan

It will be in 3.2.2.

1 Like