hengsokchamroeun / javapns

Automatically exported from code.google.com/p/javapns
0 stars 0 forks source link

Exception under heavy load (getNextThread not synchronized) #84

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
We are using the notification pool under heavy load with 5 threads. It seems 
that when it surpasses the total amount of threads it breaks.

What version of the product are you using? On what operating system?
2.1beta

Please provide any additional information below.

Guessing that the problem is that this method is not synchronized so when 
nextThread reaches a number over total threads, threads.get throws index out of 
range. It keeps throwing out of range with the number increasing by 1.

protected NotificationThread getNextThread() {
        NotificationThread thread = threads.get(nextThread++);
        if (nextThread >= threads.size()) nextThread = 0;
        return thread;
    }

java.lang.ArrayIndexOutOfBoundsException: Array index out of range: 4860
        at java.util.Vector.get(Vector.java:694)
        at javapns.notification.transmission.NotificationThreads.getNextThread(NotificationThreads.java:197)
        at javapns.notification.transmission.NotificationThreads.getNextAvailableThread(NotificationThreads.java:183)
        at javapns.notification.transmission.NotificationThreads.queue(NotificationThreads.java:170)
        at javapns.notification.transmission.NotificationThreads.queue(NotificationThreads.java:147)

Original issue reported on code.google.com by gonz...@gmail.com on 17 Nov 2011 at 3:36

GoogleCodeExporter commented 8 years ago
Indeed, the getNextThread method should have been synchronized.  I fixed in 
r328.  You can download the latest build from SVN to get the fix.

Thank you for the very precise issue report!

Original comment by sype...@gmail.com on 17 Nov 2011 at 3:55

GoogleCodeExporter commented 8 years ago
Hi! I was wrong on the correction. The problem was when all of the threads were 
busy. 
This fixed the code:

protected NotificationThread getNextAvailableThread() {
        for (int i = 0; i < threads.size(); i++) {
            NotificationThread thread = getNextThread();
            boolean busy = thread.isBusy();
            if (!busy) return thread;
        }
        return getNextThread(); /* All threads are busy, return the next one regardless of its busy status */
    }

    /**
     * Get the next thread to use.
     * 
     * @return a thread
     */
    protected NotificationThread getNextThread() {
        if (nextThread >= threads.size()) nextThread = 0;
        NotificationThread thread = threads.get(nextThread++);
        return thread;
    }

Original comment by gonz...@gmail.com on 22 Nov 2011 at 4:47