tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filip Hanik - Dev Lists <devli...@hanik.com>
Subject Re: svn commit: r763298 - in /tomcat/trunk/java/org/apache/catalina: core/StandardContext.java core/StandardHost.java tribes/membership/Membership.java util/InstanceSupport.java util/LifecycleSupport.java
Date Thu, 09 Apr 2009 22:52:09 GMT
I'm generally against this find bugs 'may be bugs' issues.
is there an actual bug here?

Filip

markt@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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>   


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


Mime
View raw message