views:

289

answers:

2

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?

A: 

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.

Klerk
I have tried releasing the semaphore there but that doesn't help, it still has the same behavior. Also, no other exception is thrown in this function. The only one possible is the InterruptedException thrown by acquire() (and I dont think i should be releasing the semaphore when the thread never acquired it in the first place).
Prashast
@Prashast: Yes, you are right. You have to make sure that you never "release" a semaphore that you did not obtain (unless you are implementing some rather weird kind of logic).
Thilo
+2  A: 

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
        }
    });
Gregory Mostizky
Thanks. That fixed my problem. I'm still curious though what was happening with my semaphore solution.
Prashast
@Prashast: is it possible that you were actually interrupting a different thread?
Stephen C
No, i'm quite certain I was interrupting the correct thread. Verified that multiple times by stepping through the code.
Prashast