commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: svn commit: r935362 - /commons/proper/pool/trunk/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java
Date Sun, 18 Apr 2010 16:19:20 GMT
I will revert this if there are objections or failures on other
JDKs.  If there are no objections or failures, I will do the same
for GKOP tests.  All current tests pass for me, but the added
synchronization may make them less sensitive, so we may wish
document lack of thread safety instead and revert.  I can see both
sides of this.

I did not fully synchronize any of the factory methods, even though
for activate and validate in their current form, the effect would be
the same. The reason for this is that we may want to add
configurable latency to these methods and we don't want the waits to
lock the factory.

Thanks in advance for review / comments or even vetos ;)

Phil

psteitz@apache.org wrote:
> Author: psteitz
> Date: Sun Apr 18 15:59:27 2010
> New Revision: 935362
> 
> URL: http://svn.apache.org/viewvc?rev=935362&view=rev
> Log:
> Made SimpleFactory threadsafe.
> 
> Modified:
>     commons/proper/pool/trunk/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java
> 
> Modified: commons/proper/pool/trunk/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java
> URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java?rev=935362&r1=935361&r2=935362&view=diff
> ==============================================================================
> --- commons/proper/pool/trunk/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java
(original)
> +++ commons/proper/pool/trunk/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java
Sun Apr 18 15:59:27 2010
> @@ -1332,26 +1332,26 @@ public class TestGenericObjectPool exten
>              evenValid = evalid;
>              oddValid = ovalid;
>          }
> -        void setValid(boolean valid) {
> +        public synchronized void setValid(boolean valid) {
>              setEvenValid(valid);
>              setOddValid(valid);            
>          }
> -        void setEvenValid(boolean valid) {
> +        public synchronized void setEvenValid(boolean valid) {
>              evenValid = valid;
>          }
> -        void setOddValid(boolean valid) {
> +        public synchronized void setOddValid(boolean valid) {
>              oddValid = valid;
>          }
> -        public void setThrowExceptionOnPassivate(boolean bool) {
> +        public synchronized void setThrowExceptionOnPassivate(boolean bool) {
>              exceptionOnPassivate = bool;
>          }
> -        public void setMaxActive(int maxActive) {
> +        public synchronized void setMaxActive(int maxActive) {
>              this.maxActive = maxActive;
>          }
> -        public void setDestroyLatency(long destroyLatency) {
> +        public synchronized void setDestroyLatency(long destroyLatency) {
>              this.destroyLatency = destroyLatency;
>          }
> -        public void setMakeLatency(long makeLatency) {
> +        public synchronized void setMakeLatency(long makeLatency) {
>              this.makeLatency = makeLatency;
>          }
>          public Object makeObject() { 
> @@ -1365,36 +1365,70 @@ public class TestGenericObjectPool exten
>              if (makeLatency > 0) {
>                  doWait(makeLatency);
>              }
> -            return String.valueOf(makeCounter++);
> +            int counter;
> +            synchronized(this) {
> +                counter = makeCounter++;
> +            }
> +            return String.valueOf(counter);
>          }
>          public void destroyObject(Object obj) throws Exception {
> -            if (destroyLatency > 0) {
> +            boolean wait;
> +            boolean hurl;
> +            synchronized(this) {
> +                wait = destroyLatency > 0;
> +                hurl = exceptionOnDestroy;
> +            }
> +            if (wait) {
>                  doWait(destroyLatency);
>              }
>              synchronized(this) {
>                  activeCount--;
>              }
> -            if (exceptionOnDestroy) {
> +            if (hurl) {
>                  throw new Exception();
>              }
>          }
>          public boolean validateObject(Object obj) {
> -            if (enableValidation) { 
> -                return validateCounter++%2 == 0 ? evenValid : oddValid; 
> +            boolean validate;
> +            boolean evenTest;
> +            boolean oddTest;
> +            int counter;
> +            synchronized(this) {
> +                validate = enableValidation;
> +                evenTest = evenValid;
> +                oddTest = oddValid;
> +                counter = validateCounter++;
> +            }
> +            if (validate) { 
> +                return counter%2 == 0 ? evenTest : oddTest; 
>              }
>              else {
>                  return true;
>              }
>          }
>          public void activateObject(Object obj) throws Exception {
> -            if (exceptionOnActivate) {
> -                if (!(validateCounter++%2 == 0 ? evenValid : oddValid)) {
> +            boolean hurl;
> +            boolean evenTest;
> +            boolean oddTest;
> +            int counter;
> +            synchronized(this) {
> +                hurl = exceptionOnActivate;
> +                evenTest = evenValid;
> +                oddTest = oddValid;
> +                counter = validateCounter++;
> +            }
> +            if (hurl) {
> +                if (!(counter%2 == 0 ? evenTest : oddTest)) {
>                      throw new Exception();
>                  }
>              }
>          }
>          public void passivateObject(Object obj) throws Exception {
> -            if(exceptionOnPassivate) {
> +            boolean hurl;
> +            synchronized(this) {
> +                hurl = exceptionOnPassivate;
> +            }
> +            if (hurl) {
>                  throw new Exception();
>              }
>          }
> @@ -1411,23 +1445,23 @@ public class TestGenericObjectPool exten
>          long makeLatency = 0;
>          int maxActive = Integer.MAX_VALUE;
>  
> -        public boolean isThrowExceptionOnActivate() {
> +        public synchronized boolean isThrowExceptionOnActivate() {
>              return exceptionOnActivate;
>          }
>  
> -        public void setThrowExceptionOnActivate(boolean b) {
> +        public synchronized void setThrowExceptionOnActivate(boolean b) {
>              exceptionOnActivate = b;
>          }
>          
> -        public void setThrowExceptionOnDestroy(boolean b) {
> +        public synchronized void setThrowExceptionOnDestroy(boolean b) {
>              exceptionOnDestroy = b;
>          }
>  
> -        public boolean isValidationEnabled() {
> +        public synchronized boolean isValidationEnabled() {
>              return enableValidation;
>          }
>  
> -        public void setValidationEnabled(boolean b) {
> +        public synchronized void setValidationEnabled(boolean b) {
>              enableValidation = b;
>          }
>          
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message