Fixed bug 2740 (again), IceBox observers fixes

5 files changed, 121 insertions, 165 deletions

diff --git a/cpp/src/IceBox/ServiceManagerI.cpp b/cpp/src/IceBox/ServiceManagerI.cpp
index 0d8710fb6cf..691dc574089 100644
--- a/cpp/src/IceBox/ServiceManagerI.cpp
+++ b/cpp/src/IceBox/ServiceManagerI.cpp
@@ -154,7 +154,6 @@ IceBox::ServiceManagerI::startService(const string& name, const Current&)
         out << "ServiceManager: unknown exception in start for service " << info.name;
     }
     
-    set<ServiceObserverPrx> observers;
     {
         IceUtil::Monitor<IceUtil::Mutex>::Lock lock(*this);
 
@@ -166,7 +165,10 @@ IceBox::ServiceManagerI::startService(const string& name, const Current&)
                 if(started)
                 {
                     p->status = Started;
-                    observers = _observers;
+
+                    vector<string> services;
+                    services.push_back(name);
+                    servicesStarted(services, _observers);
                 }
                 else
                 {
@@ -178,13 +180,6 @@ IceBox::ServiceManagerI::startService(const string& name, const Current&)
         _pendingStatusChanges = false;
         notifyAll();
     }
-
-    if(observers.size() != 0)
-    {
-        vector<string> services;
-        services.push_back(name);
-        servicesStarted(services, observers);
-    }
 }
 
 void
@@ -237,7 +232,6 @@ IceBox::ServiceManagerI::stopService(const string& name, const Current&)
         out << "ServiceManager: unknown exception in stop for service " << info.name;
     }
 
-    set<ServiceObserverPrx> observers;
     {
         IceUtil::Monitor<IceUtil::Mutex>::Lock lock(*this);
 
@@ -249,7 +243,10 @@ IceBox::ServiceManagerI::stopService(const string& name, const Current&)
                 if(stopped)
                 {
                     p->status = Stopped;
-                    observers = _observers;
+
+                    vector<string> services;
+                    services.push_back(name);
+                    servicesStopped(services, _observers);
                 }
                 else
                 {
@@ -261,13 +258,6 @@ IceBox::ServiceManagerI::stopService(const string& name, const Current&)
         _pendingStatusChanges = false;
         notifyAll();
     }
-
-    if(observers.size() != 0)
-    {
-        vector<string> services;
-        services.push_back(name);
-        servicesStopped(services, observers);
-    }
 }
 
 
@@ -299,7 +289,6 @@ IceBox::ServiceManagerI::addObserver(const ServiceObserverPrx& observer, const I
        
         if(activeServices.size() > 0)
         {
-            lock.release();
             observer->servicesStarted_async(new AMICallback<AMI_ServiceObserver_servicesStarted>(this, observer),
                                             activeServices);
         }
@@ -857,19 +846,13 @@ IceBox::ServiceManagerI::stopAll()
 
     _services.clear();
 
-    set<ServiceObserverPrx> observers = _observers;
-    lock.release();
-    servicesStopped(stoppedServices, observers);
+    servicesStopped(stoppedServices, _observers);
 }
 
 
 void
 IceBox::ServiceManagerI::servicesStarted(const vector<string>& services, const set<ServiceObserverPrx>& observers)
 {
-    //
-    // Must be called with 'this' unlocked
-    //
-
     if(services.size() > 0)
     {
         for(set<ServiceObserverPrx>::const_iterator p = observers.begin(); p != observers.end(); ++p)
@@ -884,10 +867,6 @@ IceBox::ServiceManagerI::servicesStarted(const vector<string>& services, const s
 void
 IceBox::ServiceManagerI::servicesStopped(const vector<string>& services, const set<ServiceObserverPrx>& observers)
 {
-    //
-    // Must be called with 'this' unlocked
-    //
-
     if(services.size() > 0)
     {
         for(set<ServiceObserverPrx>::const_iterator p = observers.begin(); p != observers.end(); ++p)
diff --git a/cs/src/IceBox/ServiceManagerI.cs b/cs/src/IceBox/ServiceManagerI.cs
index ce6598b8895..483f4d9cc25 100644
--- a/cs/src/IceBox/ServiceManagerI.cs
+++ b/cs/src/IceBox/ServiceManagerI.cs
@@ -134,7 +134,6 @@ class ServiceManagerI : ServiceManagerDisp_
                             e.ToString());
         }
 
-        Dictionary<ServiceObserverPrx, bool> observers = null;
         lock(this)
         {
             int i;
@@ -146,7 +145,10 @@ class ServiceManagerI : ServiceManagerDisp_
                     if(started)
                     {
                         info.status = ServiceStatus.Started;
-                        observers = new Dictionary<ServiceObserverPrx, bool>(_observers);
+
+                        List<string> services = new List<string>();
+                        services.Add(name);
+                        servicesStarted(services, _observers.Keys);
                     }
                     else
                     {
@@ -159,13 +161,6 @@ class ServiceManagerI : ServiceManagerDisp_
             _pendingStatusChanges = false;
             Monitor.PulseAll(this);
         }
-
-        if(observers != null)
-        {
-            List<string> services = new List<string>();
-            services.Add(name);
-            servicesStarted(services, observers.Keys);
-        }
     }
 
     public override void
@@ -212,7 +207,6 @@ class ServiceManagerI : ServiceManagerDisp_
                             e.ToString());
         }
 
-        Dictionary<ServiceObserverPrx, bool> observers = null;
         lock(this)
         {
             int i;
@@ -224,7 +218,10 @@ class ServiceManagerI : ServiceManagerDisp_
                     if(stopped)
                     {
                         info.status = ServiceStatus.Stopped;
-                        observers = new Dictionary<ServiceObserverPrx, bool>(_observers);
+                        
+                        List<string> services = new List<string>();
+                        services.Add(name);
+                        servicesStopped(services, _observers.Keys);
                     }
                     else
                     {
@@ -237,13 +234,6 @@ class ServiceManagerI : ServiceManagerDisp_
             _pendingStatusChanges = false;
             Monitor.PulseAll(this);
         }
-
-        if(observers != null)
-        {
-            List<string> services = new List<string>();
-            services.Add(name);
-            servicesStopped(services, observers.Keys);
-        }
     }
 
     public override void
@@ -591,7 +581,7 @@ class ServiceManagerI : ServiceManagerDisp_
             catch(System.InvalidCastException ex)
             {
                 FailureException e = new FailureException(ex);
-                e.reason = err + "InvalidCastException to Ice.PluginFactory";
+                e.reason = err + "InvalidCastException to IceBox.Service";
                 throw e;
             }
             catch(System.UnauthorizedAccessException ex)
@@ -723,9 +713,6 @@ class ServiceManagerI : ServiceManagerDisp_
     private void
     stopAll()
     {
-        List<string> stoppedServices = new List<string>();
-        Dictionary<ServiceObserverPrx, bool> observers = null;
-
         lock(this)
         {
             //
@@ -741,6 +728,7 @@ class ServiceManagerI : ServiceManagerDisp_
             // the disk. Services are stopped in the reverse order of which they were started.
             //
             _services.Reverse();
+            List<string> stoppedServices = new List<string>();
             foreach(ServiceInfo info in _services)
             {
                 if(info.status == ServiceStatus.Started)
@@ -813,10 +801,8 @@ class ServiceManagerI : ServiceManagerDisp_
             }
             
             _services.Clear();
-            observers = new Dictionary<ServiceObserverPrx, bool>(_observers);
+            servicesStopped(stoppedServices, _observers.Keys);
         }
-
-        servicesStopped(stoppedServices, observers.Keys);
     }
 
     private void
diff --git a/java/build.xml b/java/build.xml
index 317e653897a..5924d0c7970 100644
--- a/java/build.xml
+++ b/java/build.xml
@@ -230,15 +230,27 @@
     </target>
 
     <target name="icegridadmin-pro-jar" depends="icegridadmin-compile" if="build-icegridadmin-pro-jar">
-        <condition property="library.jars" value="${java.home}/../Classes/classes.jar:${java.home}/../Classes/jsse.jar">
+        <condition property="library.jarfiles" value="classes.jar,jsse.jar">
             <os family="mac"/>
         </condition>
-        <condition property="library.jars" value="${java.home}/lib/rt.jar:${java.home}/lib/jsse.jar">
+        <condition property="library.jarfiles" value="rt.jar,jsse.jar">
+            <!-- Library jar files for Sun JDK -->
             <available file="${java.home}/lib/rt.jar"/>
         </condition>
-       <condition property="library.jars" value="${java.home}/lib/vm.jar:${java.home}/lib/core.jar:${java.home}/lib/graphics.jar:${java.home}/lib/security.jar">
+        <condition property="library.jarfiles" value="vm.jar,core.jar,graphics.jar,security.jar">
+            <!-- Library jar files for IBM J9 (from Linux SuSE) -->
             <available file="${java.home}/lib/vm.jar"/>
         </condition>
+        <condition property="library.jarpath" value="${java.home}/../Classes" else="${java.home}/lib">
+            <os family="mac"/>
+        </condition>
+        <pathconvert property="library.jars">
+           <filelist dir="${library.jarpath}" files="${library.jarfiles}"/>
+           <mapper>
+              <!-- Add quotes around the patch of each Jar file (necessary for Windows) -->
+              <globmapper from="*" to="&quot;*&quot;"/>   
+           </mapper>
+        </pathconvert>
 
         <manifest file="${lib.dir}/icegridgui.mf">
             <attribute name="Main-Class" value="IceGridGUI.Main"/>
diff --git a/java/src/IceBox/ServiceManagerI.java b/java/src/IceBox/ServiceManagerI.java
index 6545cd39473..6be461c616b 100644
--- a/java/src/IceBox/ServiceManagerI.java
+++ b/java/src/IceBox/ServiceManagerI.java
@@ -93,7 +93,10 @@ public class ServiceManagerI extends _ServiceManagerDisp
                     if(started)
                     {
                         in.status = StatusStarted;
-                        observers = new java.util.HashSet<ServiceObserverPrx>(_observers);
+
+                        java.util.List<String> services = new java.util.ArrayList<String>();
+                        services.add(name);
+                        servicesStarted(services, _observers);                        
                     }
                     else
                     {
@@ -105,13 +108,6 @@ public class ServiceManagerI extends _ServiceManagerDisp
             _pendingStatusChanges = false;
             notifyAll();
         }
-
-        if(observers != null)
-        {
-            java.util.List<String> services = new java.util.ArrayList<String>();
-            services.add(name);
-            servicesStarted(services, observers);
-        }
     }
 
     public void
@@ -163,7 +159,6 @@ public class ServiceManagerI extends _ServiceManagerDisp
                             sw.toString());
         }
 
-        java.util.Set<ServiceObserverPrx> observers = null;
         synchronized(this)
         {
             java.util.Iterator<ServiceInfo> p = _services.iterator();
@@ -175,7 +170,10 @@ public class ServiceManagerI extends _ServiceManagerDisp
                     if(stopped)
                     {
                         in.status = StatusStopped;
-                        observers = new java.util.HashSet<ServiceObserverPrx>(_observers);
+
+                        java.util.List<String> services = new java.util.ArrayList<String>();
+                        services.add(name);
+                        servicesStopped(services, _observers);
                     }
                     else
                     {
@@ -187,13 +185,6 @@ public class ServiceManagerI extends _ServiceManagerDisp
             _pendingStatusChanges = false;
             notifyAll();
         }
-
-        if(observers != null)
-        {
-            java.util.List<String> services = new java.util.ArrayList<String>();
-            services.add(name);
-            servicesStopped(services, observers);
-        }
     }
 
     public void
@@ -667,134 +658,125 @@ public class ServiceManagerI extends _ServiceManagerDisp
         }
     }
 
-    private void
+    private synchronized void
     stopAll()
     {
-        java.util.List<String> stoppedServices = new java.util.ArrayList<String>();
-        java.util.Set<ServiceObserverPrx> observers = null;
+        //
+        // First wait for any active startService/stopService calls to complete.
+        //
+        while(_pendingStatusChanges)
+        {
+            try
+            {
+                wait();
+            }
+            catch(java.lang.InterruptedException ex)
+            {
+            }
+        }
 
-        synchronized(this)
+        //
+        // For each service, we call stop on the service and flush its database environment to
+        // the disk. Services are stopped in the reverse order of the order they were started.
+        //
+        java.util.List<String> stoppedServices = new java.util.ArrayList<String>();
+        java.util.ListIterator<ServiceInfo> p = _services.listIterator(_services.size());
+        while(p.hasPrevious())
         {
-            //
-            // First wait for any active startService/stopService calls to complete.
-            //
-            while(_pendingStatusChanges)
+            ServiceInfo info = p.previous();
+            if(info.status == StatusStarted)
             {
                 try
                 {
-                    wait();
+                    info.service.stop();
+                    info.status = StatusStopped;
+                    stoppedServices.add(info.name);
                 }
-                catch(java.lang.InterruptedException ex)
+                catch(java.lang.Exception e)
                 {
+                    java.io.StringWriter sw = new java.io.StringWriter();
+                    java.io.PrintWriter pw = new java.io.PrintWriter(sw);
+                    e.printStackTrace(pw);
+                    pw.flush();
+                    _logger.warning("ServiceManager: exception in stop for service " + info.name + "\n" +
+                                    sw.toString());
                 }
             }
 
-            //
-            // For each service, we call stop on the service and flush its database environment to
-            // the disk. Services are stopped in the reverse order of the order they were started.
-            //
-            java.util.ListIterator<ServiceInfo> p = _services.listIterator(_services.size());
-            while(p.hasPrevious())
+            try
             {
-                ServiceInfo info = p.previous();
-                if(info.status == StatusStarted)
-                {
-                    try
-                    {
-                        info.service.stop();
-                        info.status = StatusStopped;
-                        stoppedServices.add(info.name);
-                    }
-                    catch(java.lang.Exception e)
-                    {
-                        java.io.StringWriter sw = new java.io.StringWriter();
-                        java.io.PrintWriter pw = new java.io.PrintWriter(sw);
-                        e.printStackTrace(pw);
-                        pw.flush();
-                        _logger.warning("ServiceManager: exception in stop for service " + info.name + "\n" +
-                                        sw.toString());
-                    }
-                }
+                _server.communicator().removeAdminFacet("IceBox.Service." + info.name + ".Properties");
+            }
+            catch(Ice.LocalException e)
+            {
+                // Ignored
+            }
 
+            if(info.communicator != null)
+            {
                 try
                 {
-                    _server.communicator().removeAdminFacet("IceBox.Service." + info.name + ".Properties");
+                    info.communicator.shutdown();
+                    info.communicator.waitForShutdown();
                 }
-                catch(Ice.LocalException e)
+                catch(Ice.CommunicatorDestroyedException e)
                 {
-                    // Ignored
+                    //
+                    // Ignore, the service might have already destroyed
+                    // the communicator for its own reasons.
+                    //
                 }
-
-                if(info.communicator != null)
+                catch(java.lang.Exception e)
                 {
-                    try
-                    {
-                        info.communicator.shutdown();
-                        info.communicator.waitForShutdown();
-                    }
-                    catch(Ice.CommunicatorDestroyedException e)
-                    {
-                        //
-                        // Ignore, the service might have already destroyed
-                        // the communicator for its own reasons.
-                        //
-                    }
-                    catch(java.lang.Exception e)
-                    {
-                        java.io.StringWriter sw = new java.io.StringWriter();
-                        java.io.PrintWriter pw = new java.io.PrintWriter(sw);
-                        e.printStackTrace(pw);
-                        pw.flush();
-                        _logger.warning("ServiceManager: exception in stop for service " + info.name + "\n" +
-                                        sw.toString());
-                    }
-
-                    try
-                    {
-                        info.communicator.destroy();
-                    }
-                    catch(java.lang.Exception e)
-                    {
-                        java.io.StringWriter sw = new java.io.StringWriter();
-                        java.io.PrintWriter pw = new java.io.PrintWriter(sw);
-                        e.printStackTrace(pw);
-                        pw.flush();
-                        _logger.warning("ServiceManager: exception in stop for service " + info.name + "\n" +
-                                        sw.toString());
-                    }
+                    java.io.StringWriter sw = new java.io.StringWriter();
+                    java.io.PrintWriter pw = new java.io.PrintWriter(sw);
+                    e.printStackTrace(pw);
+                    pw.flush();
+                    _logger.warning("ServiceManager: exception in stop for service " + info.name + "\n" +
+                                    sw.toString());
                 }
-            }
 
-            if(_sharedCommunicator != null)
-            {
                 try
                 {
-                    _sharedCommunicator.destroy();
+                    info.communicator.destroy();
                 }
-                catch(Exception e)
+                catch(java.lang.Exception e)
                 {
                     java.io.StringWriter sw = new java.io.StringWriter();
                     java.io.PrintWriter pw = new java.io.PrintWriter(sw);
                     e.printStackTrace(pw);
                     pw.flush();
-                    _logger.warning("ServiceManager: unknown exception while destroying shared communicator:\n" +
+                    _logger.warning("ServiceManager: exception in stop for service " + info.name + "\n" +
                                     sw.toString());
                 }
-                _sharedCommunicator = null;
             }
+        }
 
-            _services.clear();
-            observers = new java.util.HashSet<ServiceObserverPrx>(_observers);
+        if(_sharedCommunicator != null)
+        {
+            try
+            {
+                _sharedCommunicator.destroy();
+            }
+            catch(Exception e)
+            {
+                java.io.StringWriter sw = new java.io.StringWriter();
+                java.io.PrintWriter pw = new java.io.PrintWriter(sw);
+                e.printStackTrace(pw);
+                pw.flush();
+                _logger.warning("ServiceManager: unknown exception while destroying shared communicator:\n" +
+                                sw.toString());
+            }
+            _sharedCommunicator = null;
         }
 
-        servicesStopped(stoppedServices, observers);
+        _services.clear();
+        servicesStopped(stoppedServices, _observers);
     }
 
     private void
     servicesStarted(java.util.List<String> services, java.util.Set<ServiceObserverPrx> observers)
     {
-        assert !Thread.holdsLock(this);
-
         if(services.size() > 0)
         {
             String[] servicesArray = services.toArray(new String[0]);
@@ -825,8 +807,6 @@ public class ServiceManagerI extends _ServiceManagerDisp
     private void
     servicesStopped(java.util.List<String> services, java.util.Set<ServiceObserverPrx> observers)
     {
-        assert !Thread.holdsLock(this);
-
         if(services.size() > 0)
         {
             String[] servicesArray = services.toArray(new String[0]);
diff --git a/java/src/IceInternal/Selector.java b/java/src/IceInternal/Selector.java
index 32b7f36d641..9bae64191b9 100644
--- a/java/src/IceInternal/Selector.java
+++ b/java/src/IceInternal/Selector.java
@@ -11,7 +11,6 @@ package IceInternal;
 
 public final class Selector
 {
-
     public 
     Selector(Instance instance, int timeout)
     {