Exception under heavy load (getNextThread not synchronized)

[mwkirk]

Original author: gonz...@gmail.com (November 17, 2011 15:36:20)

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: http://code.google.com/p/javapns/issues/detail?id=84

[mwkirk]

From sype...@gmail.com on November 17, 2011 15:55:14 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!

[mwkirk]

From gonz...@gmail.com on November 22, 2011 16:47:35 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;
}