0

I am trying to implement a cached thread pool in Java to read data coming in off of a bus. Everything works great... so long as I have data coming in on that bus.

If I leave the data cable disconnected, the program starts generating endless threads from the pool. I intermittently check /proc/{PID}/fd and it goes from 8 to 100+ in about an hour. Eventually the system crashes.

I am submitting a timeout value for these threads (30 seconds) and they do trigger a TimeoutException Catch which I see in my log file. Shouldn't these threads be ending when they time out?

Some code:

private static ExecutorService executorService = Executors.newCachedThreadPool();

(I later supply a callable)

long[] rawFrame;
Future<long[]> future = executorService.submit(callable);
try {
    rawFrame = future.get(timeout, timeUnit); // timeout is 30 seconds
    // Do some things with the raw frame and store as parsedFrame
    return parsedFrame;
} catch (TimeoutException e) {
    LOGGER.error("Bus timeout. Check connection");
    return null;
} finally {
    future.cancel(true);
}

I am just learning how to do concurrent processing in Java but as far as I know, these processes should be timing out but instead it appears they just sit there waiting for data on a bus that isn't spitting data.

What am I missing?

EDIT: Thanks for helping me narrow it down.

Here is my callable

private class BusCallable implements Callable<long[]> {

private long messageId;
private boolean readAnyFrame = false;

public BusCallable() {
    super();
    readAnyFrame = true;
}

public BusCallable(long id) {
    super();
    this.messageId = id;
    readAnyFrame = false;
}

@Override
public long[] call() throws Exception() {
    if (readAnyFrame) {
        return read(busInterface);
    }
    return readFrame(busInterface, messageId);
 }
lrich
  • 80
  • 9

1 Answers1

1

Calling Future.get() with a timeout will timeout the get(), not the thread (ie.whataver is in the callable's call() method). If your callable doesn't end, it will remain in the service and callables will accumulate if you keep adding more. It's in the callable's thread that you should put a timeout mechanism in place.

schmop
  • 1,440
  • 1
  • 12
  • 21
  • I am unfamiliar with how to accomplish this. Would I keep a timer in the call() method of the callable? – lrich May 09 '14 at 20:30
  • 1
    @Irich Show us your Callable for help with that. Chances are, there is one precise spot where it is waiting which is where you will want to put a timeout. There is no generic answer, it depends on what you are doing. – schmop May 09 '14 at 20:34
  • 1
    @Irich Okay but now the problem moved into those read() and readFrame() methods. If they are not of your writing and are not available with a timeout variant, you're stuck. Although that would raise the question of why you are adding new Callables since these methods wait indefinitely until there is an input (in which case no need to have more than one callable at a time - and multiple threads altogether being a poor approach to the problem). – schmop May 09 '14 at 20:57
  • I am a summer intern asked to debug some code so I'm not entirely sure why they made the decisions that they did. Would it be a better solution to a) Used a fixed thread number pool or b) not thread at all? Your help is very much appreciated. I am learning a lot – lrich May 09 '14 at 21:47
  • @Irich Err... This is turning into a debugging session. Here's the problem we've identified so far: you keep adding threads, those threads never end, hence the title of your question. Two possible (and mutually exclusive) solutions: 1.Stop adding threads 2.Make them end. Both could be valid, but we can't know without the code and the context, and asking others to analyze this for you is a bit much. My advice: investigate, choose your path, 1 or 2, post a new question if you get stuck along the way. To answer your comment: a)no good in current situation b)maybe if you ditch the timeout (sol. 1) – schmop May 09 '14 at 23:33