Permalink
Browse files

Backport an ApplicationPool race condition and invariant violation fix.

Some people reported that 'count' can go over 'max'. This is the cause,
I suspect.
  • Loading branch information...
1 parent 375f188 commit 1ad9d7930b4a91895c550579306f2369495e78b3 @FooBarWidget FooBarWidget committed Dec 5, 2008
Showing with 11 additions and 8 deletions.
  1. +8 −4 doc/ApplicationPool algorithm.txt
  2. +3 −4 ext/apache2/StandardApplicationPool.h
@@ -231,10 +231,14 @@ function spawn_or_use_existing(app_root):
count++
active++
else:
- # There are no apps for this app root.
- wait until
- (active < max) and
- (max_per_app == 0 or app_instance_count[app_root] < max_per_app)
+ # There are no apps for this app root. Wait until there's at
+ # least 1 idle instance, or until there's an empty slot in the
+ # pool, then restart this function. Restarting is necessary,
+ # because after waiting and reacquiring the lack, some other
+ # thread might already have spawned instances for this app root.
+ if (active >= max):
+ wait until _active_ has changed
+ goto beginning of function
# FIXME: there's a possible concurrency issue here. After
# waiting and having reacquired the lock, the state might have
# been completely changed, and there may now exist an instance
@@ -449,11 +449,9 @@ class StandardApplicationPool: public ApplicationPool {
activeOrMaxChanged.notify_all();
}
} else {
- while (!(
- active < max &&
- (maxPerApp == 0 || appInstanceCount[appRoot] < maxPerApp)
- )) {
+ if (active >= max) {
activeOrMaxChanged.wait(l);
+ goto beginning_of_function;
}
if (count == max) {
container = inactiveApps.front();
@@ -654,6 +652,7 @@ class StandardApplicationPool: public ApplicationPool {
appInstanceCount.clear();
count = 0;
active = 0;
+ activeOrMaxChanged.notify_all();
}
virtual void setMaxIdleTime(unsigned int seconds) {

0 comments on commit 1ad9d79

Please sign in to comment.