sebb wrote:
On 12/04/2009, Filip Hanik - Dev Lists <devli...@hanik.com> wrote:
sebb wrote:

On 10/04/2009, Mark Thomas <ma...@apache.org> wrote:


Filip Hanik - Dev Lists wrote:



I'm generally against this find bugs 'may be bugs' issues.


 > is there an actual bug here?


Reported bug, no. Bugs uses could hit, yes. Hence why this is in trunk
 and not being proposed for backport.

 Are all the syncs necessary? I haven't looked in detail but I suspect
 that right now, that most of them are not. As we increase JMX
 functionality and have more dynamic configuration then we'll almost
 certainly need them so I don't see the harm in getting this right now.


I've no idea why this is related to JMX.

Synchronization is not only about preventing lost updates, it is also
about ensuring proper publication.

If thread A writes to a non-volatile variable, thread B is only
guaranteed to see the latest copy of the variable if both thread A and
thread B use synchronization on the *same* variable.

Without synch., thread B may never see the updated variable. Indeed it
may see an  updated reference but an incomplete object.

Most of the time, a synchronized setter/unsynchronized getter will
work just fine.
However, this does not mean that it will always work.


 We *all* understand what the tool is reporting. However, the tool is not
looking at the entire picture, hence thinking the tool is right on
everything, is simple meaningless.

Of course the tool can generate false positives. However, it does find
a lot of bugs, so each report is worth investigating.

So let's look at the case of StandardContext.

If the class instances are never shared between threads, then the
existing synch. is not needed and should be removed.

If the class instances are shared between threads, then the class
needs to be thread-safe, and any existing synch. needs to be
effective. This was not the case with the methods that changed lock
objects.
this is just theorizing. Taken straight out of a book. If you were programming based on that, then every single object would be synchronized. However, understanding what the code actually does, and then realizing that some stuff even though it looks non thread safe, will never be an issue. That is Tomcat for you. It's not pretty, but it works. And we wont be making it pretty for vanity sake.

Even after fixing the lock object problems, the class on its own is
not thread-safe, however there is no Javadoc to document what locks
callers need to obtain to ensure thread-safety.

Some of the getXXX methods in the existing class used synch. and some
did not. Assuming that the class needs to be thread-safe, then at some
point synch. will need to be done to ensure proper publication.

The existing synch. regime is at best fragile, and any external
requirements should at least be documented.
To us, the code is the documentation.
 Filip



 Mark


 >
 > Filip
 >
 > ma...@apache.org wrote:
 >> Author: markt
 >> Date: Wed Apr  8 16:08:42 2009
 >> New Revision: 763298
 >>
 >> URL:
http://svn.apache.org/viewvc?rev=763298&view=rev
 >> Log:
 >> Fix
https://issues.apache.org/bugzilla/show_bug.cgi?id=46990
 >> Various sync issues.
 >>
 >> Modified:
 >>
tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
 >>
tomcat/trunk/java/org/apache/catalina/core/StandardHost.java
 >>
 >>
tomcat/trunk/java/org/apache/catalina/tribes/membership/Membership.java
 >>
tomcat/trunk/java/org/apache/catalina/util/InstanceSupport.java
 >>
tomcat/trunk/java/org/apache/catalina/util/LifecycleSupport.java
 >>
 >> Modified:
tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
 >> URL:
 >>
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardContext.java?rev=763298&r1=763297&r2=763298&view=diff
 >>
 >>
==============================================================================
 >>
 >> ---
tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
 >> (original)
 >> +++
tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
 >> Wed Apr  8 16:08:42 2009
 >> @@ -201,6 +201,8 @@
 >>       * application, in the order they were encountered in the
web.xml
 >> file.
 >>       */
 >>      private String applicationListeners[] = new String[0];
 >> +    +    private final Object applicationListenersLock = new
Object();
 >>
 >>
 >>      /**
 >> @@ -223,6 +225,8 @@
 >>      private ApplicationParameter applicationParameters[] =
 >>          new ApplicationParameter[0];
 >>
 >> +    private final Object applicationParametersLock = new Object();
 >> +
 >>      /**
 >>       * The application available flag for this Context.
 >> @@ -263,6 +267,8 @@
 >>       * The security constraints for this web application.
 >>       */
 >>      private SecurityConstraint constraints[] = new
 >> SecurityConstraint[0];
 >> +    +    private final Object constraintsLock = new Object();
 >>
 >>
 >>      /**
 >> @@ -364,6 +370,9 @@
 >>       * defined in the deployment descriptor.
 >>       */
 >>      private FilterMap filterMaps[] = new FilterMap[0];
 >> +    +    private final Object filterMapsLock = new Object();
 >> +
 >>
 >>      /**
 >>       * Filter mappings added via {...@link ServletContext} may have to
 >> be inserted
 >> @@ -388,6 +397,8 @@
 >>       */
 >>      private String instanceListeners[] = new String[0];
 >>
 >> +    private final Object instanceListenersLock = new Object();
 >> +
 >>
 >>      /**
 >>       * The login configuration descriptor for this web application.
 >> @@ -508,6 +519,8 @@
 >>       */
 >>      private String securityRoles[] = new String[0];
 >>
 >> +    private final Object securityRolesLock = new Object();
 >> +
 >>
 >>      /**
 >>       * The servlet mappings for this web application, keyed by
 >> @@ -515,6 +528,8 @@
 >>       */
 >>      private HashMap<String, String> servletMappings =
 >>          new HashMap<String, String>();
 >> +    +    private final Object servletMappingsLock = new Object();
 >>
 >>
 >>      /**
 >> @@ -559,12 +574,16 @@
 >>       */
 >>      private String watchedResources[] = new String[0];
 >>
 >> +    private final Object watchedResourcesLock = new Object();
 >> +
 >>
 >>      /**
 >>       * The welcome files for this application.
 >>       */
 >>      private String welcomeFiles[] = new String[0];
 >>
 >> +    private final Object welcomeFilesLock = new Object();
 >> +
 >>
 >>      /**
 >>       * The set of classnames of LifecycleListeners that will be
added
 >> @@ -572,6 +591,7 @@
 >>       */
 >>      private String wrapperLifecycles[] = new String[0];
 >>
 >> +    private final Object wrapperLifecyclesLock = new Object();
 >>
 >>      /**
 >>       * The set of classnames of ContainerListeners that will be
added
 >> @@ -579,6 +599,7 @@
 >>       */
 >>      private String wrapperListeners[] = new String[0];
 >>
 >> +    private final Object wrapperListenersLock = new Object();
 >>
 >>      /**
 >>       * The pathname to the work directory for this context
(relative to
 >> @@ -2021,7 +2042,7 @@
 >>       */
 >>      public void addApplicationListener(String listener) {
 >>
 >> -        synchronized (applicationListeners) {
 >> +        synchronized (applicationListenersLock) {
 >>              String results[] =new
String[applicationListeners.length
 >> + 1];
 >>              for (int i = 0; i < applicationListeners.length; i++) {
 >>                  if
(listener.equals(applicationListeners[i])) {
 >> @@ -2048,7 +2069,7 @@
 >>       */
 >>      public void
addApplicationParameter(ApplicationParameter
 >> parameter) {
 >>
 >> -        synchronized (applicationParameters) {
 >> +        synchronized (applicationParametersLock) {
 >>              String newName = parameter.getName();
 >>              for (int i = 0; i < applicationParameters.length; i++)
{
 >>                  if
 >>
(newName.equals(applicationParameters[i].getName())
&&
 >> @@ -2145,7 +2166,7 @@
 >>          }
 >>
 >>          // Add this constraint to the set for our web application
 >> -        synchronized (constraints) {
 >> +        synchronized (constraintsLock) {
 >>              SecurityConstraint results[] =
 >>                  new
SecurityConstraint[constraints.length + 1];
 >>              for (int i = 0; i < constraints.length; i++)
 >> @@ -2231,7 +2252,7 @@
 >>
 >>          validateFilterMap(filterMap);
 >>          // Add this filter mapping to our registered set
 >> -        synchronized (filterMaps) {
 >> +        synchronized (filterMapsLock) {
 >>              FilterMap results[] =new FilterMap[filterMaps.length +
1];
 >>              System.arraycopy(filterMaps, 0, results, 0,
 >> filterMaps.length);
 >>              results[filterMaps.length] = filterMap;
 >> @@ -2256,7 +2277,7 @@
 >>          validateFilterMap(filterMap);
 >>
 >>          // Add this filter mapping to our registered set
 >> -        synchronized (filterMaps) {
 >> +        synchronized (filterMapsLock) {
 >>              FilterMap results[] = new FilterMap[filterMaps.length +
1];
 >>              System.arraycopy(filterMaps, 0, results, 0,
 >> filterMapInsertPoint);
 >>              results[filterMapInsertPoint] = filterMap;
 >> @@ -2313,7 +2334,7 @@
 >>       */
 >>      public void addInstanceListener(String listener) {
 >>
 >> -        synchronized (instanceListeners) {
 >> +        synchronized (instanceListenersLock) {
 >>              String results[] =new
String[instanceListeners.length + 1];
 >>              for (int i = 0; i < instanceListeners.length; i++)
 >>                  results[i] = instanceListeners[i];
 >> @@ -2456,7 +2477,7 @@
 >>       */
 >>      public void addSecurityRole(String role) {
 >>
 >> -        synchronized (securityRoles) {
 >> +        synchronized (securityRolesLock) {
 >>              String results[] =new String[securityRoles.length + 1];
 >>              for (int i = 0; i < securityRoles.length; i++)
 >>                  results[i] = securityRoles[i];
 >> @@ -2507,7 +2528,7 @@
 >>
(sm.getString("standardContext.servletMap.pattern",
 >> pattern));
 >>
 >>          // Add this mapping to our registered set
 >> -        synchronized (servletMappings) {
 >> +        synchronized (servletMappingsLock) {
 >>              String name2 = servletMappings.get(pattern);
 >>              if (name2 != null) {
 >>                  // Don't allow more than one servlet on the same
pattern
 >> @@ -2551,7 +2572,7 @@
 >>       */
 >>      public void addWatchedResource(String name) {
 >>
 >> -        synchronized (watchedResources) {
 >> +        synchronized (watchedResourcesLock) {
 >>              String results[] = new String[watchedResources.length +
1];
 >>              for (int i = 0; i < watchedResources.length; i++)
 >>                  results[i] = watchedResources[i];
 >> @@ -2570,7 +2591,7 @@
 >>       */
 >>      public void addWelcomeFile(String name) {
 >>
 >> -        synchronized (welcomeFiles) {
 >> +        synchronized (welcomeFilesLock) {
 >>              // Welcome files from the application deployment
descriptor
 >>              // completely replace those from the default
conf/web.xml
 >> file
 >>              if (replaceWelcomeFiles) {
 >> @@ -2597,7 +2618,7 @@
 >>       */
 >>      public void addWrapperLifecycle(String listener) {
 >>
 >> -        synchronized (wrapperLifecycles) {
 >> +        synchronized (wrapperLifecyclesLock) {
 >>              String results[] =new
String[wrapperLifecycles.length + 1];
 >>              for (int i = 0; i < wrapperLifecycles.length; i++)
 >>                  results[i] = wrapperLifecycles[i];
 >> @@ -2617,7 +2638,7 @@
 >>       */
 >>      public void addWrapperListener(String listener) {
 >>
 >> -        synchronized (wrapperListeners) {
 >> +        synchronized (wrapperListenersLock) {
 >>              String results[] =new String[wrapperListeners.length +
1];
 >>              for (int i = 0; i < wrapperListeners.length; i++)
 >>                  results[i] = wrapperListeners[i];
 >> @@ -2649,7 +2670,7 @@
 >>              wrapper = new StandardWrapper();
 >>          }
 >>
 >> -        synchronized (instanceListeners) {
 >> +        synchronized (instanceListenersLock) {
 >>              for (int i = 0; i < instanceListeners.length; i++) {
 >>                  try {
 >>                      Class<?> clazz =
 >> Class.forName(instanceListeners[i]);
 >> @@ -2663,7 +2684,7 @@
 >>              }
 >>          }
 >>
 >> -        synchronized (wrapperLifecycles) {
 >> +        synchronized (wrapperLifecyclesLock) {
 >>              for (int i = 0; i < wrapperLifecycles.length; i++) {
 >>                  try {
 >>                      Class<?> clazz =
 >> Class.forName(wrapperLifecycles[i]);
 >> @@ -2678,7 +2699,7 @@
 >>              }
 >>          }
 >>
 >> -        synchronized (wrapperListeners) {
 >> +        synchronized (wrapperListenersLock) {
 >>              for (int i = 0; i < wrapperListeners.length; i++) {
 >>                  try {
 >>                      Class<?> clazz =
Class.forName(wrapperListeners[i]);
 >> @@ -2713,7 +2734,9 @@
 >>       */
 >>      public ApplicationParameter[] findApplicationParameters() {
 >>
 >> -        return (applicationParameters);
 >> +        synchronized (applicationParametersLock) {
 >> +            return (applicationParameters);
 >> +        }
 >>
 >>      }
 >>
 >> @@ -2829,7 +2852,9 @@
 >>       */
 >>      public String[] findInstanceListeners() {
 >>
 >> -        return (instanceListeners);
 >> +        synchronized (instanceListenersLock) {
 >> +            return (instanceListeners);
 >> +        }
 >>
 >>      }
 >>
 >> @@ -2987,7 +3012,7 @@
 >>       */
 >>      public boolean findSecurityRole(String role) {
 >>
 >> -        synchronized (securityRoles) {
 >> +        synchronized (securityRolesLock) {
 >>              for (int i = 0; i < securityRoles.length; i++) {
 >>                  if (role.equals(securityRoles[i]))
 >>                      return (true);
 >> @@ -3004,7 +3029,9 @@
 >>       */
 >>      public String[] findSecurityRoles() {
 >>
 >> -        return (securityRoles);
 >> +        synchronized (securityRolesLock) {
 >> +            return (securityRoles);
 >> +        }
 >>
 >>      }
 >>
 >> @@ -3017,7 +3044,7 @@
 >>       */
 >>      public String findServletMapping(String pattern) {
 >>
 >> -        synchronized (servletMappings) {
 >> +        synchronized (servletMappingsLock) {
 >>              return (servletMappings.get(pattern));
 >>          }
 >>
 >> @@ -3030,7 +3057,7 @@
 >>       */
 >>      public String[] findServletMappings() {
 >>
 >> -        synchronized (servletMappings) {
 >> +        synchronized (servletMappingsLock) {
 >>              String results[] = new
String[servletMappings.size()];
 >>              return
 >>
(servletMappings.keySet().toArray(results));
 >> @@ -3113,7 +3140,7 @@
 >>       */
 >>      public boolean findWelcomeFile(String name) {
 >>
 >> -        synchronized (welcomeFiles) {
 >> +        synchronized (welcomeFilesLock) {
 >>              for (int i = 0; i < welcomeFiles.length; i++) {
 >>                  if (name.equals(welcomeFiles[i]))
 >>                      return (true);
 >> @@ -3129,7 +3156,9 @@
 >>       * defined, a zero length array will be returned.
 >>       */
 >>      public String[] findWatchedResources() {
 >> -        return watchedResources;
 >> +        synchronized (watchedResourcesLock) {
 >> +            return watchedResources;
 >> +        }
 >>      }
 >>           @@ -3139,7 +3168,9 @@
 >>       */
 >>      public String[] findWelcomeFiles() {
 >>
 >> -        return (welcomeFiles);
 >> +        synchronized (welcomeFilesLock) {
 >> +            return (welcomeFiles);
 >> +        }
 >>
 >>      }
 >>
 >> @@ -3150,7 +3181,9 @@
 >>       */
 >>      public String[] findWrapperLifecycles() {
 >>
 >> -        return (wrapperLifecycles);
 >> +        synchronized (wrapperLifecyclesLock) {
 >> +            return (wrapperLifecycles);
 >> +        }
 >>
 >>      }
 >>
 >> @@ -3161,7 +3194,9 @@
 >>       */
 >>      public String[] findWrapperListeners() {
 >>
 >> -        return (wrapperListeners);
 >> +        synchronized (wrapperListenersLock) {
 >> +            return (wrapperListeners);
 >> +        }
 >>
 >>      }
 >>
 >> @@ -3220,7 +3255,7 @@
 >>       */
 >>      public void removeApplicationListener(String
listener) {
 >>
 >> -        synchronized (applicationListeners) {
 >> +        synchronized (applicationListenersLock) {
 >>
 >>              // Make sure this welcome file is currently present
 >>              int n = -1;
 >> @@ -3260,7 +3295,7 @@
 >>       */
 >>      public void removeApplicationParameter(String
name) {
 >>
 >> -        synchronized (applicationParameters) {
 >> +        synchronized (applicationParametersLock) {
 >>
 >>              // Make sure this parameter is currently present
 >>              int n = -1;
 >> @@ -3319,7 +3354,7 @@
 >>       */
 >>      public void
removeConstraint(SecurityConstraint constraint) {
 >>
 >> -        synchronized (constraints) {
 >> +        synchronized (constraintsLock) {
 >>
 >>              // Make sure this constraint is currently present
 >>              int n = -1;
 >> @@ -3399,7 +3434,7 @@
 >>       */
 >>      public void removeFilterMap(FilterMap filterMap) {
 >>
 >> -        synchronized (filterMaps) {
 >> +        synchronized (filterMapsLock) {
 >>
 >>              // Make sure this filter mapping is currently present
 >>              int n = -1;
 >> @@ -3438,7 +3473,7 @@
 >>       */
 >>      public void removeInstanceListener(String listener) {
 >>
 >> -        synchronized (instanceListeners) {
 >> +        synchronized (instanceListenersLock) {
 >>
 >>              // Make sure this welcome file is currently present
 >>              int n = -1;
 >> @@ -3550,7 +3585,7 @@
 >>       */
 >>      public void removeSecurityRole(String role) {
 >>
 >> -        synchronized (securityRoles) {
 >> +        synchronized (securityRolesLock) {
 >>
 >>              // Make sure this security role is currently present
 >>              int n = -1;
 >> @@ -3589,7 +3624,7 @@
 >>      public void removeServletMapping(String pattern) {
 >>
 >>          String name = null;
 >> -        synchronized (servletMappings) {
 >> +        synchronized (servletMappingsLock) {
 >>              name =
servletMappings.remove(pattern);
 >>          }
 >>          Wrapper wrapper = (Wrapper) findChild(name);
 >> @@ -3624,7 +3659,7 @@
 >>       */
 >>      public void removeWatchedResource(String name) {
 >>          -        synchronized (watchedResources) {
 >> +        synchronized (watchedResourcesLock) {
 >>
 >>              // Make sure this watched resource is currently present
 >>              int n = -1;
 >> @@ -3661,7 +3696,7 @@
 >>       */
 >>      public void removeWelcomeFile(String name) {
 >>
 >> -        synchronized (welcomeFiles) {
 >> +        synchronized (welcomeFilesLock) {
 >>
 >>              // Make sure this welcome file is currently present
 >>              int n = -1;
 >> @@ -3701,7 +3736,7 @@
 >>      public void removeWrapperLifecycle(String listener) {
 >>
 >>
 >> -        synchronized (wrapperLifecycles) {
 >> +        synchronized (wrapperLifecyclesLock) {
 >>
 >>              // Make sure this welcome file is currently present
 >>              int n = -1;
 >> @@ -3740,7 +3775,7 @@
 >>      public void removeWrapperListener(String listener) {
 >>
 >>
 >> -        synchronized (wrapperListeners) {
 >> +        synchronized (wrapperListenersLock) {
 >>
 >>              // Make sure this welcome file is currently present
 >>              int n = -1;
 >> @@ -4701,7 +4736,9 @@
 >>          // Notify our interested LifecycleListeners
 >>
lifecycle.fireLifecycleEvent(DESTROY_EVENT, null);
 >>
 >> -        instanceListeners = new String[0];
 >> +        synchronized (instanceListenersLock) {
 >> +            instanceListeners = new String[0];
 >> +        }
 >>
 >>      }
 >>
 >> Modified:
tomcat/trunk/java/org/apache/catalina/core/StandardHost.java
 >> URL:
 >>
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardHost.java?rev=763298&r1=763297&r2=763298&view=diff
 >>
 >>
==============================================================================
 >>
 >> ---
tomcat/trunk/java/org/apache/catalina/core/StandardHost.java
 >> (original)
 >> +++
tomcat/trunk/java/org/apache/catalina/core/StandardHost.java
Wed
 >> Apr  8 16:08:42 2009
 >> @@ -72,6 +72,8 @@
 >>       * The set of aliases for this Host.
 >>       */
 >>      private String[] aliases = new String[0];
 >> +    +    private final Object aliasesLock = new Object();
 >>
 >>
 >>      /**
 >> @@ -514,20 +516,19 @@
 >>
 >>          alias = alias.toLowerCase();
 >>
 >> -        // Skip duplicate aliases
 >> -        for (int i = 0; i < aliases.length; i++) {
 >> -            if (aliases[i].equals(alias))
 >> -                return;
 >> +        synchronized (aliasesLock) {
 >> +            // Skip duplicate aliases
 >> +            for (int i = 0; i < aliases.length; i++) {
 >> +                if (aliases[i].equals(alias))
 >> +                    return;
 >> +            }
 >> +            // Add this alias to the list
 >> +            String newAliases[] = new String[aliases.length + 1];
 >> +            for (int i = 0; i < aliases.length; i++)
 >> +                newAliases[i] = aliases[i];
 >> +            newAliases[aliases.length] = alias;
 >> +            aliases = newAliases;
 >>          }
 >> -
 >> -        // Add this alias to the list
 >> -        String newAliases[] = new String[aliases.length + 1];
 >> -        for (int i = 0; i < aliases.length; i++)
 >> -            newAliases[i] = aliases[i];
 >> -        newAliases[aliases.length] = alias;
 >> -
 >> -        aliases = newAliases;
 >> -
 >>          // Inform interested listeners
 >>          fireContainerEvent(ADD_ALIAS_EVENT,
alias);
 >>
 >> @@ -556,7 +557,9 @@
 >>       */
 >>      public String[] findAliases() {
 >>
 >> -        return (this.aliases);
 >> +        synchronized (aliasesLock) {
 >> +            return (this.aliases);
 >> +        }
 >>
 >>      }
 >>
 >> @@ -631,7 +634,7 @@
 >>
 >>          alias = alias.toLowerCase();
 >>
 >> -        synchronized (aliases) {
 >> +        synchronized (aliasesLock) {
 >>
 >>              // Make sure this alias is currently present
 >>              int n = -1;
 >> @@ -766,7 +769,9 @@
 >>       }
 >>
 >>      public String[] getAliases() {
 >> -        return aliases;
 >> +        synchronized (aliasesLock) {
 >> +            return aliases;
 >> +        }
 >>      }
 >>
 >>      private boolean initialized=false;
 >>
 >> Modified:
 >>
tomcat/trunk/java/org/apache/catalina/tribes/membership/Membership.java
 >> URL:
 >>
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/tribes/membership/Membership.java?rev=763298&r1=763297&r2=763298&view=diff
 >>
 >>
==============================================================================
 >>
 >> ---
 >>
tomcat/trunk/java/org/apache/catalina/tribes/membership/Membership.java
 >> (original)
 >> +++
 >>
tomcat/trunk/java/org/apache/catalina/tribes/membership/Membership.java
 >> Wed Apr  8 16:08:42 2009
 >> @@ -41,6 +41,8 @@
 >>  {
 >>      protected static final MemberImpl[] EMPTY_MEMBERS = new
 >> MemberImpl[0];
 >>      +    private final Object membersLock = new Object();
 >> +         /**
 >>       * The name of this membership, has to be the same as the name
 >> for the local
 >>       * member
 >> @@ -63,7 +65,7 @@
 >>      protected Comparator<Member> memberComparator = new
 >> MemberComparator();
 >>
 >>      public Object clone() {
 >> -        synchronized (members) {
 >> +        synchronized (membersLock) {
 >>              Membership clone = new Membership(local,
memberComparator);
 >>              clone.map = (HashMap<MemberImpl, MbrEntry>)
map.clone();
 >>              clone.members = new MemberImpl[members.length];
 >> @@ -139,7 +141,7 @@
 >>       * @param member The member to add
 >>       */
 >>      public synchronized MbrEntry addMember(MemberImpl member) {
 >> -      synchronized (members) {
 >> +      synchronized (membersLock) {
 >>            MbrEntry entry = new MbrEntry(member);
 >>            if (!map.containsKey(member) ) {
 >>                map.put(member, entry);
 >> @@ -160,7 +162,7 @@
 >>       */
 >>      public void removeMember(MemberImpl member) {
 >>          map.remove(member);
 >> -        synchronized (members) {
 >> +        synchronized (membersLock) {
 >>              int n = -1;
 >>              for (int i = 0; i < members.length; i++) {
 >>                  if (members[i] == member ||
members[i].equals(member)) {
 >>
 >> Modified:
tomcat/trunk/java/org/apache/catalina/util/InstanceSupport.java
 >> URL:
 >>
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/InstanceSupport.java?rev=763298&r1=763297&r2=763298&view=diff
 >>
 >>
==============================================================================
 >>
 >> ---
tomcat/trunk/java/org/apache/catalina/util/InstanceSupport.java
 >> (original)
 >> +++
tomcat/trunk/java/org/apache/catalina/util/InstanceSupport.java
 >> Wed Apr  8 16:08:42 2009
 >> @@ -64,6 +64,8 @@
 >>       * The set of registered InstanceListeners for event
notifications.
 >>       */
 >>      private InstanceListener listeners[] = new InstanceListener[0];
 >> +    +    private final Object listenersLock = new Object(); // Lock
 >> object for changes to listeners
 >>
 >>
 >>      /**
 >> @@ -95,7 +97,7 @@
 >>       */
 >>      public void
addInstanceListener(InstanceListener listener) {
 >>
 >> -      synchronized (listeners) {
 >> +      synchronized (listenersLock) {
 >>            InstanceListener results[] =
 >>              new InstanceListener[listeners.length
+ 1];
 >>            for (int i = 0; i < listeners.length; i++)
 >> @@ -312,7 +314,7 @@
 >>       */
 >>      public void
removeInstanceListener(InstanceListener listener) {
 >>
 >> -        synchronized (listeners) {
 >> +        synchronized (listenersLock) {
 >>              int n = -1;
 >>              for (int i = 0; i < listeners.length; i++) {
 >>                  if (listeners[i] == listener) {
 >>
 >> Modified:
 >>
tomcat/trunk/java/org/apache/catalina/util/LifecycleSupport.java
 >> URL:
 >>
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/LifecycleSupport.java?rev=763298&r1=763297&r2=763298&view=diff
 >>
 >>
==============================================================================
 >>
 >> ---
tomcat/trunk/java/org/apache/catalina/util/LifecycleSupport.java
 >> (original)
 >> +++
tomcat/trunk/java/org/apache/catalina/util/LifecycleSupport.java
 >> Wed Apr  8 16:08:42 2009
 >> @@ -66,6 +66,8 @@
 >>       * The set of registered LifecycleListeners for event
notifications.
 >>       */
 >>      private LifecycleListener listeners[] = new
LifecycleListener[0];
 >> +    +    private final Object listenersLock = new Object(); // Lock
 >> object for changes to listeners
 >>
 >>
 >>      //
---------------------------------------------------------
 >> Public Methods
 >> @@ -78,7 +80,7 @@
 >>       */
 >>      public void
addLifecycleListener(LifecycleListener listener) {
 >>
 >> -      synchronized (listeners) {
 >> +      synchronized (listenersLock) {
 >>            LifecycleListener results[] =
 >>              new LifecycleListener[listeners.length
+ 1];
 >>            for (int i = 0; i < listeners.length; i++)
 >> @@ -126,7 +128,7 @@
 >>       */
 >>      public void
removeLifecycleListener(LifecycleListener listener) {
 >>
 >> -        synchronized (listeners) {
 >> +        synchronized (listenersLock) {
 >>              int n = -1;
 >>              for (int i = 0; i < listeners.length; i++) {
 >>                  if (listeners[i] == listener) {
 >>
 >>
 >>
 >>
---------------------------------------------------------------------
 >> To unsubscribe, e-mail:
dev-unsubscr...@tomcat.apache.org
 >> For additional commands, e-mail: dev-h...@tomcat.apache.org
 >>
 >>
 >>
 >
 >
 >
---------------------------------------------------------------------
 > To unsubscribe, e-mail:
dev-unsubscr...@tomcat.apache.org
 > For additional commands, e-mail: dev-h...@tomcat.apache.org
 >




---------------------------------------------------------------------
 To unsubscribe, e-mail:
dev-unsubscr...@tomcat.apache.org
 For additional commands, e-mail: dev-h...@tomcat.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org




---------------------------------------------------------------------
 To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
 For additional commands, e-mail: dev-h...@tomcat.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to