views:

573

answers:

4

I'm trying to understand how to ensure that a specific action completes in a certain amount of time. Seems like a simple job for java's new util.concurrent library. However, this task claims a connection to the database and I want to be sure that it properly releases that connection upon timeout.

so to call the service:

int resultCount = -1;
ExecutorService executor = null;
try {
 executor = Executors.newSingleThreadExecutor();
 FutureTask<Integer> task = new CopyTask<Integer>();
 executor.execute(task);
 try {
  resultCount = task.get(2, TimeUnit.MINUTES);
 } catch (Exception e) {
   LOGGER.fatal("Migrate Events job crashed.", e);
   task.cancel(true);
   return;
 }
} finally {
if (executor != null) {
 executor.shutdown();
}

The task itself simply wrapps a callable, here is the call method:

@Override
public Integer call() throws Exception {
 Session session = null;
 try {
  session = getSession();
  ... execute sql against sesssion ...
  }
 } finally {
  if (session != null) {
   session.release();
  }
 }
}

So, my question for those who've made it this far, is: Is session.release() garaunteed to be called in the case that the task fails due to a TimeoutException? I postulate that it is no, but I would love to be proven wrong.

Thanks

edit: The problem I'm having is that occasionally the sql in question is not finishing due to wierd db problems. So, what I want to do is simply close the connection, let the db rollback the transaction, get some rest and reattempt this at a later time. So I'm treating the get(...) as if it were like killing the thead. Is that wrong?

A: 

You cannot interrupt a thread from the outside, so the timeout will have no effect on the code down in the JDBC layer (perhaps even over in JNI-land somewhere.) Presumably eventually the SQL work will end and the session.release() will happen, but that may be long after the end of your timeout.

djna
A: 

Your example says:

copyRecords.cancel(true);

I assume this was meant to say:

task.cancel(true);

Your finally block will be called assuming that the contents of the try block are interruptible. Some operations are (like wait()), some operations are not (like InputStream#read()). It all depends on the operation that that the code is blocking on when the task is interrupted.

Kevin
+1  A: 

The finally block will eventually execute.

When your Task takes longer then 2 minutes, a TimeoutException is thrown but the actual thread continues to perform it's work and eventually it will call the finally block. Even if you cancel the task and force an interrupt, the finally block will be called.

Here's a small example based in your code. You can test these situations:

public static void main(String[] args) {
 int resultCount = -1;
 ExecutorService executor = null;
 try {
  executor = Executors.newSingleThreadExecutor();
  FutureTask<Integer> task = new FutureTask<Integer>(new Callable<Integer>() {
   @Override
   public Integer call() throws Exception {
    try {
     Thread.sleep(10000);
     return 1;
    } finally {
     System.out.println("FINALLY CALLED!!!");
    }
   }
  });
  executor.execute(task);
  try {
   resultCount = task.get(1000, TimeUnit.MILLISECONDS);
  } catch (Exception e) {
   System.out.println("Migrate Events job crashed: " + e.getMessage());
   task.cancel(true);
   return;
  }
 } finally {
  if (executor != null) {
   executor.shutdown();
  }
 }
}
bruno conde
The problem with this code is that my analog to: Thread.sleep, may never finish. Yet I still want to see 'Finally Called'
Nathan Feger
+3  A: 

When you call task.get() with a timeout, that timeout only applies to the attempt to obtain the results (in your current thread), not the calculation itself (in the worker thread). Hence your problem here; if a worker thread gets into some state from which it will never return, then the timeout simply ensures that your polling code will keep running but will do nothing to affect the worker.

Your call to task.cancel(true) in the catch block is what I was initially going to suggest, and this is good coding practice. Unfortunately this only sets a flag on the thread that may/should be checked by well-behaved long-running, cancellable tasks, but it doesn't take any direct action on the other thread. If the SQL executing methods don't declare that they throw InterruptedException, then they aren't going to check this flag and aren't going to be interruptable via the typical Java mechanism.

Really all of this comes down to the fact that the code in the worker thread must support some mechanism of stopping itself if it's run for too long. Supporting the standard interrupt mechanism is one way of doing this; checking some boolean flag intermittently, or other bespoke alternatives, would work too. However there is no guaranteed way to cause another thread to return (short of Thread.stop, which is deprecated for good reason). You need to coordinate with the running code to signal it to stop in a way that it will notice.

In this particular case, I expect there are probably some parameters you could set on the DB connection so that the SQL calls will time out after a given period, meaning that control returns to your Java code (probably with some exception) and so the finally block gets called. If not, i.e. there's no way to make the database call (such as PreparedStatement.execute()) return control after some predetermined time, then you'll need to spawn an extra thread within your Callable that can monitor a timeout and forcibly close the connection/session if it expires. This isn't very nice though and your code will be a lot cleaner if you can get the SQL calls to cooperate.

(So ironically despite you supplying a good amount of code to support this question, the really important part is the bit you redacted: "... execute sql against sesssion ..." :-))

Andrzej Doyle
Thanks this is a great comment. I didn't event think about taking advantage of the connection timeout. Thanks
Nathan Feger