Part 2: synchronization.
Consider a case where you have a huge number of simultaneous requests for CheckRevision(). If you try to process all of them at the same time, you're most likely going to cause every one of the people who requested CheckRevision() to timeout on battle.net. So, the idea of a queue comes in to play.
Queues are great if you're at an amusement park. Queues are not great when you have a maximum time to live (TTL) --- for this scenario, I'm giong to say that is 30s. You do not want to do CheckRevision()s all at once because everyone will die, and you do not want to do CheckRevision()s one at a time, because you'll finish the request for fewer than the maximum possible number of people in the window.
The bottom line is that you will never be able to eliminate the case where someone can timeout. For this reason, you should time how long someone is waiting for CheckRevision() to start. If they are waiting 20s for it to start with a 30s TTL, just close their socket - it's a lost cause. By not attempting to do a CheckRevision() for the person who can't use it, you're making it more likely that the next person in line will have a chance to succeed. The bot might even detect the socket is closed in time to abort their bnet connection, thus avoiding the ridiculous ip ban that bnet gives for taking too long to complete CheckRevision().
Now, on to how to design a queue. I won't bore you with how to choose or construct a queue class, but I will bore you with several strategies for using them.
The first, and simplest:
static Queue<ConnectionThread> queue;
void waitInLineForCheckRevision() {
queue.add(this);
while(queue.contains(this))
sleep();
// we're done
}
Seems nice, and allows you to have a few worker threads, but as we've already concluded, that's not the goal, and what you'll find if you try to actually implement this method is that you're going to hit synchronization problems that a synchronized() block won't solve. A more complex design pattern including semaphore-y constructs would solve the synchronization problem, but this model still is pretty bad. It can't accommodate very well the case where you want to give up on waiting for a CheckRevision() result and close the socket, because the worker thread can't remove the item from the queue until it's finished. This argument pretty much applies to any use of worker threads, though you could hack your way around it.
[ IF YOU KNOW WHAT SEMAPHORES ARE START SKIPPING NOW ]
I mentioned semaphores in the previous paragraph, but if you don't know what they are, here's a brief summary:
private static int LIST_SEMAPHORE = 0;
private static List<Object> myList = new List<Object>();
void synchronizedIteration() {
while(LIST_SEMAPHORE > 0)
sleep();
LIST_SEMAPHORE++;
for(Object o : myList)
someOperation();
LIST_SEMAPHORE--;
}
void synchronizedAdd(Object o) {
while(LIST_SEMAPHORE > 0)
sleep();
LIST_SEMAPHORE++;
myList.add(o);
LIST_SEMAPHORE--;
}
This code tries to prevent multiple threads from accessing the list at the same time using a semaphore. You might see that there is a flaw in this code, which is that the thread could yield between the end of the while loop and the increment operation, causing the undesired behavior to occur anyways. That's an excellent argument, but the reality is that threads don't yield very often when they don't explicitly ask to yield, and the fact that the threads will be sleep()ing if the list is under heavy pressure will pretty much guarantee that that the thread won't yield before it gets a chance to increment the semaphore. There is always that corner case, though.
[ STOP SKIPPING NOW ]
Synchronization is a really nasty thing to have to worry about. It's always better to avoid having to deal with it at all than to deal with it correctly; it's rare that you can plug every hole in a sinking ship. There is something to be admired about semaphores, though: it allows you to limit [to a number greater than one] the number of threads that can do a certain operation at one time without creating extra unnecessary worker threads. This is PERFECT for what you are doing. You want to limit the number of simultanious CheckRevision()s to a pretty low number, but that number isn't really all that well defined, so it's not a big deal if an extra one falls through.
Consider:
private static int CHECKREVISION_SEMAPHORE = 0;
public static ? CheckRevision() {
while(CHECKREVISION_SEMAPHORE > 2) // I have a dual-core machine, so I want 3 threads to do CheckRevision() concurrently
sleep();
CHECKREVISION_SEMAPHORE++;
? result = null;
try {
result = doCheckRevision();
} catch(Throwable t) {
// handle the exception
}
CHECKREVISION_SEMAPHORE--;
return result;
}
Remember, it's absolutely imperative that you catch everything before decrementing your semaphore, or it will just keep going up until no threads are ever allowed to doCheckRevision()
Well, I think I've written enough for one day. I'm going to take a nap.