Uploaded image for project: 'Felix'
  1. Felix
  2. FELIX-5435

Service does not get loaded with updated properties that have been modified on file system after felix framework restart

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Blocker
    • Resolution: Fixed
    • configadmin-1.8.0, configadmin-1.8.8, configadmin-1.8.12
    • configadmin-1.8.14
    • Configuration Admin
    • None
    • Java 1.7; Using Felix framework version 5.4.0.

    Description

      In felix.scr 2.0.6, seeing an issue where a configuration for a particular Service is not loaded.

      We have one @Component(Componenent A) that offers one service, lets call this Service A.

      ComponenetA attributes are as follows:

      (name="ServiceA", policy= ConfigurationPolicy.REQUIRE, metatype=true, immediate=true) 

      ServiceA has a configuration that could look like this:

      {
      	"name" : "ServiceA",
      	"description" : "Displays which version it has of ServiceB",
      	"versionRequired" : "1"
      }
      

      We have a second @Component(Component B) that offers another service, lets call that Service B.

      The Component attributes for Component B are as follows:

      (name:"ServiceB", policy=ConfigurationPolicy.OPTIONAL, metatype=true, immediate=true).

      (I understand with immediate=true and ConfiguraitonPolicy.OPTIONAL, Service B will be initialized without any configuration when it first gets started by the framework, however if there is configuration for it on the file system, Felix DS will reload Service B with what is on file system when it scans the directory for ServiceB configuraiton).

      Service B, has some properties in the configuraiton for it, that are used by ServiceA if ServiceB has been loaded with configuration. For example say we have a configuration for ServiceB that looks like this.

      {
      	"name" : "ServiceB",
      	"description" : "is used by ServiceA"
      	"version" : "1"
      }
      

      Service A has a reference to Service B, as so:

      @Reference(policy = ReferencePolicy.DYNAMIC)
      volatile ServiceB serviceB = null;
      
      For example if we have something like this for ComponentA/ServiceA, I will keep the code short to illustrate the problem. 
      	@Component(name = "ServiceA",
              policy = ConfigurationPolicy.REQUIRE,
              metatype = true,
              description = "ComponentA with ServiceA",
              immediate = true)
      	@Service(value = {ServiceAInterface.class})
      	public class ServiceA implements ServiceAInterface {
      
      		private String version; 
      		private String description; 
      		private Json config = null;
      
      		// version requested by VersionB 
      		private String versionRequested;
      
      		@Reference(policy = ReferencePolicy.DYNAMIC)
      		volatile ServiceB serviceB;
      
      		@Activate
      		protected void activate(ComponentContext context) {
      			this.config = getConfigFromComponentContext(context);
      			this.description = config.get("description");
      			this.versionRequested = config.get("versionRequested");
      
      			// get the version that ServiceA config requests 
      			serviceB.getVersionAsStringAsync(versionRequested).thenOnResult(
      				this.version = versionFromServiceB; 
      			);
      		}
      
      		// @Deactivate method would go here
      
      		@override
      		public String getVersion() {
      			if (version != null) {
      				return version;
      			} else {
      				return "No Version has been set yet!";
      			}
      		}		
      
      	}
      

      For the scenario where this problem exists we need a configuration for both ServiceA and ServiceB. The configuration is stored on the file system in some directory that felix watches to load changes as they occur. /tmp/felixConfig/ ServiceA.json ServiceB.json.

      Start up the Felix OSGi environment with the configuration and what I see is that it works as expected. ServiceA waits until it has its @Refernce to ServiceB satisfied before it attempts to activate as expected. Then if I made a call to ServiceA#getVersion("1"), and ServiceB is up and running with configuration from the file that was found, ServiceA will be able to get the version.

      Here is where the problem is. I shutdown the OSGi Framework. Then I change the configuration to both ServiceA and ServiceB, "versionRequired" : "2" and "version" : "2", respectively. I then start the OSGi framework back up and now ServiceA does not load correctly in the ComponentRegistry as "versionRequired" : "2", but rather has the old version, "1". I see that felix.scr picked up the changes and is attempting to notify all the ConfigurationListener(org.osgi.service.cm.ConfigurationListener) implementations.

      The setup I am working with picks up a change and makes a call to org.apache.felix.cm.impl.ConfigurationAdapter#update,

      	@Override 
          public void update( Dictionary<String, ?> properties ) throws IOException
          {
              delegatee.getConfigurationManager().log( LogService.LOG_DEBUG, "update(properties={0})", new Object[]
                  { properties } );
      
              checkActive();
              checkDeleted();
              delegatee.update( properties );
          }
      
      ** An Important NOTE **
      I set a breakpoint at `delegatee.update(properties)`. When I look here at the `configurations` inside the ConfigurationAdapter->ConfigurationAdmin->configurations, the configuration for ServiceA is not here! The configuration for ServiceB is in that list of configurations. IF I look at a previous version of felix.scr, 2.0.2, I see ServiceA in that list of configurations..it no longer is there, in version 2.0.4,2.0.6, and trunk and this will result in the problem because the revision number will not be updated.
      **      **
      

      These properties are the new properties for ServiceA, that has the the versionRequested attribute in the ServiceA object updated to versionRequest = 2, because I changed it on the file system when the system was down and on startup of the framework it pick up those changes on the file system and is attempting to propagate this configuration to all necessary services that require it.

      Digging deeper in the call to delegatee.update( properties), is implemented in org.apache.felix.cm.impl#update(Dictionary<String, ?> properties) as so,

        * @see org.osgi.service.cm.Configuration#update(java.util.Dictionary)
           */
          public void update( Dictionary<String, ?> properties ) throws IOException
          {
              PersistenceManager localPersistenceManager = getPersistenceManager();
              if ( localPersistenceManager != null )
              {
                  CaseInsensitiveDictionary newProperties = new CaseInsensitiveDictionary( properties );
      
                  getConfigurationManager().log( LogService.LOG_DEBUG, "Updating config {0} with {1}", new Object[]
                      { getPidString(), newProperties } );
      
                  setAutoProperties( newProperties, true );
      
                  // persist new configuration
                  localPersistenceManager.store( getPidString(), newProperties );
      
                  // finally assign the configuration for use
                  configure( newProperties );
      
                  // if this is a factory configuration, update the factory with
                  // do this only after configuring with current properties such
                  // that a concurrently registered ManagedServiceFactory service
                  // does not receive a new/unusable configuration
                  updateFactory();
      
                  // update the service and fire an CM_UPDATED event
                  getConfigurationManager().updated( this, true );
              }
          }
      

      The intresting thing here is that the reference now to 'this' ConfigurationImpl seen on the code above `getConfigurationManager().updated( this, true );`
      has the new revision, and properties of the ConfigurationImpl, but later on when we dig deeper in the call to getConfigurationManager().updated( this, true );
      we will see that we cannot find the ConfigurationImpl(with the UPDATED revision and properties) in the list of Configurations of the ConfirurationAdmin.

      Following the getConfigurationManager().updated( this, true ); code brings us to where the ConfigurationManager perpares the eventTread and the UpdateThread as so:

      org.apache.felix.cm.impl.ConfigurationManager

      	void updated( ConfigurationImpl config, boolean fireEvent )
          {
              if ( fireEvent )
              {
                  fireConfigurationEvent( ConfigurationEvent.CM_UPDATED, config.getPidString(), config.getFactoryPidString() );
              }
              updateThread.schedule( new UpdateConfiguration( config ) ); // this will occur with the correct revision...
              log( LogService.LOG_DEBUG, "UpdateConfiguration({0}) scheduled", new Object[]
                  { config.getPid() } );
          }
      

      One very important thing to note here is that the `ConfigurationImpl config` passed into this updated(config, fireEvent) method has the correct `revision` and `properties`.
      That ConfigurationImpl is not passed into the `fireConfigurationEvent`, but we later look it up in the ConfigAdmin...which we will see that it is not there, and this is what I believe to be the root of the problem.

      Before we get to the updateThread.schedule(), lets go ahead into the fireConfiguraitonEvent() which creates a new FireConfigurationEvent object for both asyncSender and for
      syncSender, I did not have any syncSender so that could be ignored. This new object gets scheduled in the `eventThread`. This thread will then loop through each ConfigurationListener
      and passes the ConfiguraitonEvent that it had created letting them know about the changed.

      Using from the sample I wrote above, ComponentA and all its properties are part of this ConfigurationEvent. The listener that is having problems actually doing anything with the ConfigurationEvent is the

      org.apache.felix.scr.impl.manager.RegionConfigurationSupport#configurationEvent(ConfigurationEvent event)

      when we look at the code inside the implementation of that we see this line that is returning
      a ConfigurationInfo that has a revision that is revision=1, and the properties are the updatedProperties

      {version=2,etc.} instead of returning revision=2,updatedProperties{version=2,etc.}

      around like 259

      final ConfigurationInfo configInfo = getConfigurationInfo( pid, targetedPid, // This IS where the config returns the wrong revision
                                      componentHolder, bundleContext );
      

      If we dig deeper to understand why the ConfigurationInfo comes back with the correct updated properties but without updating the revision to 2 we will inside

      org.apache.felix.scr.impl.manager.RegionConfigurationSupport#getConfigurationInfo(final TargetedPID pid, TargetedPID targetedPid, ComponentHolder<?> componentHolder, final BundleContext bundleContext)

          try
              {//final ServiceReference caRef = bundleContext.getServiceReference(ComponentRegistry.CONFIGURATION_ADMIN);
                  final ConfigurationAdmin ca = getConfigAdmin( bundleContext );
                  try  // this looks interesting, could be getting the wrong config admin? ?
                  {
                      Configuration[] configs = ca.listConfigurations( filter( pid.getRawPid() ) ); <----- this ca.listConfigurations() is the problem, like I mentioned earlier in the ** NOTE **
                      if ( configs != null && configs.length > 0 )
                      {
                          Configuration config = configs[0];
                          return new ConfigurationInfo( config.getProperties(), config.getBundleLocation(), 
                              config.getChangeCount() );
                      }
                      ... the rest of the implementation
      

      The problem I am seeing is the the `ConfigurationAdmin ca` does not have the configuration in its cachedConfigs

           // the cache of Configuration instances mapped by their PID
          // have this always set to prevent NPE on bundle shutdown
          private final HashMap<String, ConfigurationImpl> configurations = new HashMap<String, ConfigurationImpl>();
      

      when it makese the call to ca.listConfigurations..

       				// ensure the service.pid and returned a cached config if available
                      ConfigurationImpl cfg = getCachedConfiguration( pid ); <----- this returns null
                      if ( cfg == null )
                      {
                          cfg = new ConfigurationImpl( this, pmList[i], config ); 
                      }
      

      it returns no configuraiton from the cache, so then we return a new ConfigurationImpl which configures a NEW implementation of `ConfigurationImpl` from what we stored in the PersistanceManager above
      at org.apache.felix.cm.impl#update(Dictionary<String, ?> properties), but the revision, because it's a NEW object, is set to 1. and when this goes back up the stack and gets to

      org.apache.felix.scr.impl.manager.ConfigurableComponentHolder#configurationUpdated()

      The logical statement inside,

                      if (oldChangeCount != null && changeCount <= oldChangeCount && factoryPid.equals(oldTargetedPID)) {
                      	return false;
                      }
      

      is TRUE so the code immediately returns false and the Component(ComponentA) is not reconfigured with the new properties.

      This DOES NOT happen on version felix.scr 2.0.2, but tracing through that code I see a lot of code that has been refactored. The RegionConfigurationSupport has taken over what was the ConfigurationSupport. The revision changeCount has been refactored, and the CA has usage has also been refactored.

      It is broken in both 2.0.4, and 2.0.6.

      I did a manual bisect until I was able to narrow it down, here is a snapshot of when the behavior was introduced:

      ------------------------------------------------------------------------
      r1747831 | djencks | 2016-06-10 18:14:36 -0700 (Fri, 10 Jun 2016) | 1 line   <------------- TEST HERE [   BROKEN  ]
      
      FELIX-5270 Don't set bundle location on configurations 
      ------------------------------------------------------------------------
      

      This only started occuring after the first commit to

      FELIX-5270

      Here a larger snapshot of the actual bisect steps I took:

      FELIX-5270 log when bundle locations are inconsistent
      ------------------------------------------------------------------------
      r1749929 | djencks | 2016-06-23 08:57:30 -0700 (Thu, 23 Jun 2016) | 1 line   <------------- TEST HERE [   BROKEN  ] AT THIS REVISION 
      
      FELIX-5248 missing license header
      ------------------------------------------------------------------------
      r1749927 | cziegeler | 2016-06-23 08:48:00 -0700 (Thu, 23 Jun 2016) | 1 line
      
      Revert rev 1749869
      ------------------------------------------------------------------------
      r1749869 | cziegeler | 2016-06-23 04:45:33 -0700 (Thu, 23 Jun 2016) | 1 line 
      
      add missing license header
      ------------------------------------------------------------------------
      r1748287 | djencks | 2016-06-13 10:14:22 -0700 (Mon, 13 Jun 2016) | 1 line  
      
      FELIX-5248 test for complaint
      ------------------------------------------------------------------------
      r1747831 | djencks | 2016-06-10 18:14:36 -0700 (Fri, 10 Jun 2016) | 1 line   <------------- TEST HERE [   BROKEN  ] WINNER!!!
      
      FELIX-5270 Don't set bundle location on configurations 						
      ------------------------------------------------------------------------
      r1747329 | djencks | 2016-06-07 17:14:12 -0700 (Tue, 07 Jun 2016) | 1 line   <------------- TEST HERE [ WORKS ]
      
      FELIX-5276 track service event before changing service properties
      ------------------------------------------------------------------------
      r1746618 | djencks | 2016-06-02 12:36:26 -0700 (Thu, 02 Jun 2016) | 1 line    <------------- TEST HERE [   WORKS   ]
      
      FELIX-4417 Improve logging of circular references.  Fix some problems introduced with rev 1744827 when activate changes service properties.
      ------------------------------------------------------------------------
      r1746617 | djencks | 2016-06-02 12:36:22 -0700 (Thu, 02 Jun 2016) | 1 line    
      
      FELIX-5264 Introduce a single State enum and use an atomic to track it, and use some optimistic locking on state changes.  This fixes the specific issue found and should provide much easier diagnosis of any remaining or new problems.
      ------------------------------------------------------------------------
      r1746471 | gnodet | 2016-06-01 07:36:32 -0700 (Wed, 01 Jun 2016) | 1 line <-------- TEST HERE [ WORKS ]
      
      [FELIX-5243] Make ComponentContextImpl#setImplementationAccessible public, similar to setImplementationObject
      ------------------------------------------------------------------------
      r1746470 | gnodet | 2016-06-01 07:20:03 -0700 (Wed, 01 Jun 2016) | 1 line
      
      [FELIX-5243] Remove anonymous inner class, add a unit test to ensure package consistency
      

      Attachments

        1. FELIX_5435.patch
          4 kB
          Alin Brici

        Issue Links

          Activity

            People

              cziegeler Carsten Ziegeler
              alinbrici Alin Brici
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: