Author: mpetria
Date: Thu Aug 13 13:16:49 2015
New Revision: 1695707
URL: http://svn.apache.org/r1695707
Log:
SLING-4895: Service registry should not be called from within synchronized block
Modified:
sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/Mapping.java
sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
Modified:
sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/Mapping.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/Mapping.java?rev=1695707&r1=1695706&r2=1695707&view=diff
==============================================================================
---
sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/Mapping.java
(original)
+++
sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/Mapping.java
Thu Aug 13 13:16:49 2015
@@ -117,9 +117,6 @@ class Mapping implements Comparable<Mapp
int result = compare(this.serviceName, o.serviceName);
if (result == 0) {
result = compare(this.subServiceName, o.subServiceName);
- if (result == 0) {
- result = compare(this.userName, o.userName);
- }
}
return result;
}
Modified:
sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java?rev=1695707&r1=1695706&r2=1695707&view=diff
==============================================================================
---
sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
(original)
+++
sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
Thu Aug 13 13:16:49 2015
@@ -24,7 +24,6 @@ import java.util.Collections;
import java.util.Dictionary;
import java.util.HashMap;
import java.util.Hashtable;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
@@ -107,7 +106,7 @@ public class ServiceUserMapperImpl imple
private final List<ServiceUserValidator> validators = new
CopyOnWriteArrayList<ServiceUserValidator>();
- private SortedMap<Mapping, ServiceRegistration> activeMappingRegistrations
= new TreeMap<Mapping, ServiceRegistration>();
+ private SortedMap<Mapping, Registration> activeRegistrations = new
TreeMap<Mapping, Registration>();
private BundleContext bundleContext;
@@ -131,18 +130,26 @@ public class ServiceUserMapperImpl imple
this.globalServiceUserMappings = mappings.toArray(new
Mapping[mappings.size()]);
this.defaultUser =
PropertiesUtil.toString(config.get(PROP_DEFAULT_USER),
PROP_DEFAULT_USER_DEFAULT);
+
+ RegistrationSet registrationSet = null;
synchronized ( this.amendments ) {
this.bundleContext = bundleContext;
- this.updateMappings();
+ registrationSet = this.updateMappings();
}
+
+ this.executeServiceRegistrations(registrationSet);
}
@Deactivate
void deactivate() {
- synchronized ( this.amendments) {
- updateServiceMappings(new ArrayList<Mapping>());
+ RegistrationSet registrationSet = null;
+
+ synchronized ( this.amendments ) {
+ updateServiceRegistrations(new Mapping[0]);
bundleContext = null;
}
+
+ this.executeServiceRegistrations(registrationSet);
}
/**
@@ -173,27 +180,32 @@ public class ServiceUserMapperImpl imple
protected void bindAmendment(final MappingConfigAmendment amendment, final
Map<String, Object> props) {
final Long key = (Long) props.get(Constants.SERVICE_ID);
+ RegistrationSet registrationSet = null;
synchronized ( this.amendments ) {
amendments.put(key, amendment);
- this.updateMappings();
+ registrationSet = this.updateMappings();
}
+
+ executeServiceRegistrations(registrationSet);
}
protected void unbindAmendment(final MappingConfigAmendment amendment,
final Map<String, Object> props) {
final Long key = (Long) props.get(Constants.SERVICE_ID);
+ RegistrationSet registrationSet = null;
synchronized ( this.amendments ) {
if ( amendments.remove(key) != null ) {
- this.updateMappings();
+ registrationSet = this.updateMappings();
}
}
+ executeServiceRegistrations(registrationSet);
}
protected void updateAmendment(final MappingConfigAmendment amendment,
final Map<String, Object> props) {
this.bindAmendment(amendment, props);
}
- protected void updateMappings() {
+ protected RegistrationSet updateMappings() {
final List<MappingConfigAmendment> sortedMappings = new
ArrayList<MappingConfigAmendment>();
for(final MappingConfigAmendment amendment : this.amendments.values()
) {
sortedMappings.add(amendment);
@@ -212,43 +224,94 @@ public class ServiceUserMapperImpl imple
activeMappings = mappings.toArray(new Mapping[mappings.size()]);
- updateServiceMappings(mappings);
+ RegistrationSet registrationSet =
updateServiceRegistrations(activeMappings);
+
+ return registrationSet;
+
}
- void updateServiceMappings(final List<Mapping> newMappings) {
+ RegistrationSet updateServiceRegistrations(final Mapping[] newMappings) {
+ RegistrationSet result = new RegistrationSet();
// do not do anything if not activated
if (bundleContext == null) {
- return;
+ return result;
}
- final SortedSet<Mapping> orderedActiveMappings = new
TreeSet<Mapping>(newMappings);
+ final SortedSet<Mapping> orderedNewMappings = new
TreeSet<Mapping>(Arrays.asList(newMappings));
+ final SortedMap<Mapping, Registration> newRegistrations = new
TreeMap<Mapping, Registration>();
+
+ // keep those that are still mapped
+ for (Map.Entry<Mapping, Registration> registrationEntry:
activeRegistrations.entrySet()) {
+ boolean keepEntry = true;
- final Iterator<Map.Entry<Mapping, ServiceRegistration>> it =
activeMappingRegistrations.entrySet().iterator();
- while (it.hasNext()) {
- final Map.Entry<Mapping, ServiceRegistration> registrationEntry =
it.next();
+ if (!orderedNewMappings.contains(registrationEntry.getKey())) {
+ Registration registration = registrationEntry.getValue();
- if (!orderedActiveMappings.contains(registrationEntry.getKey())) {
- registrationEntry.getValue().unregister();
- it.remove();
+ // remove it only if it is a completed registration
+ if (registration.serviceRegistration != null) {
+ result.removed.add(registration);
+ keepEntry = false;
+ }
+ }
+
+ if (keepEntry) {
+ newRegistrations.put(registrationEntry.getKey(),
registrationEntry.getValue());
}
}
+ // add those that are new
+ for (final Mapping mapping: orderedNewMappings) {
+ if (!newRegistrations.containsKey(mapping)) {
+ Registration registration = new Registration(mapping, null);
+ newRegistrations.put(mapping, registration);
+ result.added.add(registration);
+ }
+ }
+
+ activeRegistrations = newRegistrations;
+
+ return result;
+ }
+
+ private void executeServiceRegistrations(RegistrationSet registrationSet) {
- for (final Mapping mapping: orderedActiveMappings) {
- if (!activeMappingRegistrations.containsKey(mapping)) {
- final Dictionary<String, Object> properties = new
Hashtable<String, Object>();
- if (mapping.getSubServiceName() != null) {
- properties.put(ServiceUserMapped.SUBSERVICENAME,
mapping.getSubServiceName());
+ if (registrationSet == null) {
+ return;
+ }
+
+ for (Registration registration : registrationSet.removed) {
+ if (registration.serviceRegistration != null) {
+ try {
+ registration.serviceRegistration.unregister();
+ } catch (IllegalStateException e) {
+ log.error("cannot unregister ServiceUserMapped {}",
registration.mapping, e);
}
+ registration.serviceRegistration = null;
+ }
+ }
+
+ BundleContext savedBundleContext = bundleContext;
+
+ if (savedBundleContext == null) {
+ return;
+ }
- properties.put(Mapping.SERVICENAME, mapping.getServiceName());
- final ServiceRegistration registration =
bundleContext.registerService(ServiceUserMappedImpl.SERVICEUSERMAPPED,
- new ServiceUserMappedImpl(), properties);
- activeMappingRegistrations.put(mapping, registration);
+ for (Registration registration : registrationSet.added) {
+ Mapping mapping = registration.mapping;
+ final Dictionary<String, Object> properties = new
Hashtable<String, Object>();
+ if (mapping.getSubServiceName() != null) {
+ properties.put(ServiceUserMapped.SUBSERVICENAME,
mapping.getSubServiceName());
}
+
+ properties.put(Mapping.SERVICENAME, mapping.getServiceName());
+ final ServiceRegistration serviceRegistration =
savedBundleContext.registerService(ServiceUserMappedImpl.SERVICEUSERMAPPED,
+ new ServiceUserMappedImpl(), properties);
+
+ registration.serviceRegistration = serviceRegistration;
}
+
}
private String internalGetUserId(final String serviceName, final String
subServiceName) {
@@ -294,5 +357,20 @@ public class ServiceUserMapperImpl imple
List<Mapping> getActiveMappings() {
return Collections.unmodifiableList(Arrays.asList(activeMappings));
}
+
+ class Registration {
+ private Mapping mapping;
+ private ServiceRegistration serviceRegistration;
+
+ Registration(Mapping mapping, ServiceRegistration serviceRegistration)
{
+ this.mapping = mapping;
+ this.serviceRegistration = serviceRegistration;
+ }
+ }
+
+ class RegistrationSet {
+ List<Registration> added = new ArrayList<Registration>();
+ List<Registration> removed = new ArrayList<Registration>();
+ }
}
