Reputation: 5675
I have a semaphore which restricts users to download n number of files at a time. Each file is downloaded in a separate thread.
EDIT: Modified the example so that the release happens correctly
import java.util.concurrent.Semaphore;
public void downloadFile() {
Thread downloadThread = new Thread() {
boolean bSemaphoreAcquired = false;
public void run() {
try {
semaphore.acquire();
bSemaphoreAcquired = true;
// do the download
} catch (InterruptedException e) {
} finally {
if (bSemaphoreAcquired) semaphore.release();
}
}
};
// add download thread to the list of download threads
downloadThread.start();
}
Now, any new downloads will wait for the previous downloads to finish once all the permits of the semaphore have been acquired.
When the user chooses to cancel a download that is waiting at the acquire call I call the interrupt() method to terminate that download thread. The problem I am facing is that once this semaphore has been interrupted it would not throw the InterruptedException exception the second time!! Any new thread that is created will just wait on the acquire method for ever!
Sequence of events (Semaphore that permits 2 threads)
- thread 1 downloading
- thread 2 downloading
- thread 3 waiting on acquire()
- thread 3 is cancelled (interrupt() is called). InterruptedException is thrown and the thread exits
- thread 4 is created and is now waiting on acquire()
- thread 4 is cancelled (interrupt() is called). InterruptedException IS NOT THROWN ANYMORE!!!
This is the stack trace of thread 4
Semaphore$FairSync(AbstractQueuedSynchronizer).fullGetFirstQueuedThread() line: 1276
Semaphore$FairSync(AbstractQueuedSynchronizer).getFirstQueuedThread() line: 1232
Semaphore$FairSync.tryAcquireShared(int) line: 201
Semaphore$FairSync(AbstractQueuedSynchronizer).acquireSharedInterruptibly(int) line: 1142
Semaphore.acquire() line: 267
FileDownloadManager$1.run() line: 150
Why does thread 4 not receive the exception?
Upvotes: 2
Views: 2280
Reputation: 7261
I would suggest using standard thread pool instead of semaphores. The problem with your solution is that you create a new thread regardless of whether you reached max limit or not. So if you have 1000 simultaneous requests you will create 1000 threads which is really wasteful.
Instead try something like this:
ExecutorService executorService = Executors.newFixedThreadPool(5);
executorService.submit(new Runnable() {
public void run() {
// do download
}
});
Upvotes: 2
Reputation: 2018
Are you releasing the semaphore within the catch?
Why not put the try-catch within the aquire-release. Not sure what java does, but wouldnt that be more logical. That way any problem within the try...catch always ends with the semaphore being released.
Upvotes: 0