You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by sebb <se...@gmail.com> on 2009/04/12 00:20:39 UTC
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
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.
>
> Mark
>
>
> >
> > 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
> >
>
>
>
> ---------------------------------------------------------------------
> 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
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
Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
sebb wrote:
> On 12/04/2009, Filip Hanik - Dev Lists <de...@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
>>>> >
>>>> > 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
>>>> >
>>>>
>>>>
>>>>
>>>>
>>>>
>> ---------------------------------------------------------------------
>>
>>>> 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
>>>
>>>
>>>
>>>
>>>
>> ---------------------------------------------------------------------
>> 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
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
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
Posted by sebb <se...@gmail.com>.
On 12/04/2009, Filip Hanik - Dev Lists <de...@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.
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.
> Filip
>
>
>
> >
> >
> > > Mark
> > >
> > >
> > > >
> > > > 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
> > > >
> > >
> > >
> > >
> > >
> ---------------------------------------------------------------------
> > > 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
> >
> >
> >
> >
>
>
> ---------------------------------------------------------------------
> 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
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
Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
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.
Filip
>
>> Mark
>>
>>
>> >
>> > 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
>> >
>>
>>
>>
>> ---------------------------------------------------------------------
>> 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
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org