Mailing-List: contact dev-help@cxf.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cxf.apache.org
Received-SPF: pass (athena.apache.org: domain of amichai2@amichais.net
 designates 192.115.134.103 as permitted sender)
Message-ID: <1936941496.311363896080408.JavaMail.root@shefa>
MIME-Version: 1.0
Date: Thu, 21 Mar 2013 22:01:20 +0200
From: "A. Rothman" <amichai2@amichais.net>
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:17.0) Gecko/20130308 Thunderbird/17.0.4
To: dev@cxf.apache.org
Subject: Re: Fwd: DOSGi - service not exported consistently
References: <5149FD5E.5080001@amichais.net>
 <747078278.161363803902570.JavaMail.root@shefa>
In-Reply-To: <747078278.161363803902570.JavaMail.root@shefa>
Content-Type: multipart/mixed;
 boundary="------------000901010101060803080404"

--------------000901010101060803080404
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


> 3. After all the fixes above, the export success rate is definitely 
> better than 50%, but still isn't 100% - every once in a while the 
> export still fails. Looking through the logs, I found the message 
> "RemoteServiceAdmin Implementation is shutting down now" - during 
> Karaf startup! I'm still not sure what exactly is going on, but it 
> appears that the RSA is started, services exported properly etc., but 
> then it suddenly commits suicide a few moments later with no other 
> error or indication of who/what initiated the shutdown or why. The 
> initial theory is that the dsw Activator might be getting a 
> ConfigurationAdmin updated event with null configuration, which would 
> trigger a shutdown - but I haven't yet confirmed that this is the 
> case, nor why it would happen or what the proper behavior in such a 
> case should be.
Following up on this, the config updated callback was indeed the 
problem. There's also a race condition in the class that can cause 
trouble, and an endless loop bug when the configuration does change... 
attached is a fix for the issues I've found - again, I'm new to all 
this, so please correct anything I've missed.

These are the changes:

- separated stop/start (when bundle is started or stopped) from 
init/uninit (setting up/tearing down the services when configuration 
changes) - solves endless loop caused by start re-registering managed 
service, which re-triggers update, which calls start etc. ad infinitum
- synchronized init/uninit/update (since update is called from another 
thread and so init/uninit may be called concurrently with undefined results
- fixed updated callback shutting down service right after it starts 
(when config is null), causing service exports to fail sometimes
- added proper unregistering of managed service
- added a few debug logs to help follow the sequence of events


Also, if there's anything else I should do to get these patches accepted 
and committed for the next release, please let me know!

p.s.
Sometime between 1.4 and 1.5-SNAPSHOT it appears the default exported 
service urls (with -> without "cxf" prefix) and ports (8181 -> 9000) 
have changed - is this on purpose? Actually, there are two different 
default configurations in two different places, HttpServiceManager and 
Pojo something, and now one of them is used instead of the other. 
Perhaps they should be rearranged to make it clear how and when each is 
used.


--------------000901010101060803080404
Content-Type: text/x-diff;
 name="fix_activator_config_and_sync_issues.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="fix_activator_config_and_sync_issues.diff"

Index: dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/Activator.java
===================================================================
--- dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/Activator.java	(revision 1457734)
+++ dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/Activator.java	(working copy)
@@ -56,22 +56,24 @@
     private static final String CONFIG_SERVICE_PID = "cxf-dsw";
     private ServiceRegistration rsaFactoryReg;
     private ServiceRegistration decoratorReg;
+    private ServiceRegistration managedReg;
     private IntentTracker intentTracker;
     private BundleContext bc;
 
     public void start(BundleContext bundlecontext) throws Exception {
+        LOG.debug("RemoteServiceAdmin Implementation is starting up");
         this.bc = bundlecontext;
-        start(bundlecontext, new Hashtable<String, Object>());
+        // Disable the fast infoset as it's not compatible (yet) with OSGi
+        System.setProperty("org.apache.cxf.nofastinfoset", "true");
+        init(new Hashtable<String, Object>());
+        managedReg = registerManagedService(bc);
     }
 
-    private void start(BundleContext bundlecontext, Map<String, Object> config) {
-        this.bc = bundlecontext;
+    private synchronized void init(Map<String, Object> config) {
+        LOG.debug("RemoteServiceAdmin Implementation is initializing");
         String httpBase = (String) config.get(org.apache.cxf.dosgi.dsw.Constants.HTTP_BASE);
         String cxfServletAlisas = (String) config.get(org.apache.cxf.dosgi.dsw.Constants.CXF_SERVLET_ALIAS);
-        // Disable the fast infoset as it's not compatible (yet) with OSGi
-        System.setProperty("org.apache.cxf.nofastinfoset", "true");
 
-        registerManagedService(bc);
         IntentMap intentMap = new IntentMap(new DefaultIntentMapFactory().create());
         intentTracker = new IntentTracker(bc, intentMap);
         intentTracker.open();
@@ -102,44 +104,47 @@
         return l.toArray(new String[] {});
     }
 
-    private void registerManagedService(BundleContext bundlecontext) {
+    private ServiceRegistration registerManagedService(BundleContext bundlecontext) {
         Dictionary<String, String> props = new Hashtable<String, String>();
         props.put(Constants.SERVICE_PID, CONFIG_SERVICE_PID);
-        bundlecontext.registerService(ManagedService.class.getName(), this, props);
+        return bundlecontext.registerService(ManagedService.class.getName(), this, props);
     }
 
     public void stop(BundleContext context) throws Exception {
         LOG.debug("RemoteServiceAdmin Implementation is shutting down now");
-        if (intentTracker != null) {
-            intentTracker.close();
-            // This also triggers the unimport and unexport of the remote services
-            rsaFactoryReg.unregister();
-            decoratorReg.unregister();
-            // shutdown the CXF Bus -> Causes also the shutdown of the embedded HTTP server
-            Bus b = BusFactory.getDefaultBus();
-            if (b != null) {
-                LOG.debug("Shutting down the CXF Bus");
-                b.shutdown(true);
-            }
-            intentTracker = null;
-            rsaFactoryReg = null;
-            decoratorReg = null;
+        managedReg.unregister();
+        managedReg = null;
+        uninit();
+    }
+
+    private synchronized void uninit() {
+        LOG.debug("RemoteServiceAdmin Implementation is uninitializing");
+        intentTracker.close();
+        // This also triggers the unimport and unexport of the remote services
+        rsaFactoryReg.unregister();
+        decoratorReg.unregister();
+        // shutdown the CXF Bus -> Causes also the shutdown of the embedded HTTP server
+        Bus b = BusFactory.getDefaultBus();
+        if (b != null) {
+            LOG.debug("Shutting down the CXF Bus");
+            b.shutdown(true);
         }
+        intentTracker = null;
+        rsaFactoryReg = null;
+        decoratorReg = null;
     }
 
     @SuppressWarnings({ "rawtypes", "unchecked" })
     public synchronized void updated(Dictionary config) throws ConfigurationException {
-        if (rsaFactoryReg != null) {
+        LOG.debug("RemoteServiceAdmin Implementation configuration is updated with {}", config);
+        if (config != null) {
             try {
-                stop(bc);
+                uninit();
             } catch (Exception e) {
                 LOG.error(e.getMessage(), e);
             }
+            init(getMapFromDictionary(config));
         }
-        if (config != null) {
-            Map<String, Object> configMap = getMapFromDictionary(config);
-            start(bc, configMap);
-        }
     }
 
     private Map<String, Object> getMapFromDictionary(Dictionary<String, Object> config) {
@@ -154,6 +159,4 @@
         }
         return configMap;
     }
-
-
 }

--------------000901010101060803080404--