Skip to content

Commit

Permalink
[Fix] Fixed race conditions in DefaultRuntimeRegistry, removed Regist…
Browse files Browse the repository at this point in the history
…ry.exists(String, Class) method

the exists(String, Class) method could be misleading, as an IO with the given ID might exist, but has a different type. This would not make adding an IO with the given ID possible and would lead to exceptions later on.
  • Loading branch information
eitch committed Feb 8, 2024
1 parent a23db73 commit 971ad30
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 89 deletions.
9 changes: 0 additions & 9 deletions pi4j-core/src/main/java/com/pi4j/registry/Registry.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,6 @@
* @version $Id: $Id
*/
public interface Registry extends Describable {

/**
* <p>exists.</p>
*
* @param id a {@link java.lang.String} object.
* @param type a {@link java.lang.Class} object.
* @return a boolean.
*/
boolean exists(String id, Class<? extends IO> type);
/**
* <p>exists.</p>
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,6 @@ private DefaultRegistry(RuntimeRegistry registry) {
this.registry = registry;
}

/** {@inheritDoc} */
@Override
public boolean exists(String id, Class<? extends IO> type) {
return registry.exists(id, type);
}

/** {@inheritDoc} */
@Override
public boolean exists(String id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* This file is part of the Pi4J project. More information about
* this project can be found here: https://pi4j.com/
* **********************************************************************
*
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand Down Expand Up @@ -45,9 +45,9 @@
*/
public class DefaultRuntimeRegistry implements RuntimeRegistry {

private Logger logger = LoggerFactory.getLogger(this.getClass());
private Runtime runtime = null;
private Map<String, IO> instances = new ConcurrentHashMap<>();
private static final Logger logger = LoggerFactory.getLogger(DefaultRuntimeRegistry.class);
private final Runtime runtime;
private final Map<String, IO> instances;

// static singleton instance
/**
Expand All @@ -62,91 +62,76 @@ public static RuntimeRegistry newInstance(Runtime runtime){

// private constructor
private DefaultRuntimeRegistry(Runtime runtime) {
// forbid object construction

// set local runtime reference
this.instances = new ConcurrentHashMap<>();
this.runtime = runtime;
}
}

/** {@inheritDoc} */
@Override
public RuntimeRegistry add(IO instance) throws IOInvalidIDException, IOAlreadyExistsException {

// validate target I/O instance id
String _id = validateId(instance.id());

// first test to make sure this id does not already exist in the registry
if(instances.containsKey(_id))
throw new IOAlreadyExistsException(_id);
synchronized (this.instances) {
if (instances.containsKey(_id))
throw new IOAlreadyExistsException(_id);

// add instance to collection
instances.put(_id, instance);
}

// add instance to collection
instances.put(_id, instance);
return this;
}

/** {@inheritDoc} */
@Override
public <T extends IO> T get(String id, Class<T> type) throws IOInvalidIDException, IONotFoundException {
String _id = validateId(id);

// first test to make sure this id is included in the registry
if(!instances.containsKey(_id))
T t = (T) instances.get(_id);
if(t == null)
throw new IONotFoundException(_id);

return (T)instances.get(_id);
return t;
}

/** {@inheritDoc} */
@Override
public <T extends IO> T get(String id) throws IOInvalidIDException, IONotFoundException {
String _id = validateId(id);

// first test to make sure this id is included in the registry
if(!instances.containsKey(_id))
T t = (T) instances.get(_id);
if(t == null)
throw new IONotFoundException(_id);

return (T)instances.get(_id);
return t;
}

/** {@inheritDoc} */
@Override
public <T extends IO> T remove(String id) throws IONotFoundException, IOInvalidIDException, IOShutdownException {
String _id = validateId(id);
IO shutdownInstance = null;

// first test to make sure this id is included in the registry
if(!exists(_id))
throw new IONotFoundException(_id);
synchronized (this.instances) {
String _id = validateId(id);

// shutdown instance
try {
shutdownInstance = instances.get(_id);
shutdownInstance.shutdown(runtime.context());
}
catch (LifecycleException e){
logger.error(e.getMessage(), e);
throw new IOShutdownException(shutdownInstance, e);
}
// remove the instance from the registry
IO io = this.instances.remove(_id);
if (io == null)
throw new IONotFoundException(_id);

// remove the shutdown instance from the registry
this.instances.remove(_id);
// shutdown instance
try {
io.shutdown(runtime.context());
} catch (LifecycleException e) {
logger.error(e.getMessage(), e);
throw new IOShutdownException(io, e);
}

// return the shutdown I/O provider instances
return (T)shutdownInstance;
// return the shutdown I/O provider instances
return (T) io;
}
}

/** {@inheritDoc} */
@Override
public boolean exists(String id) {
String _id = null;
try {
_id = validateId(id);
// return 'false' if the requested ID is not found
// return 'true' if the requested ID is found
return instances.containsKey(_id);
} catch (IOInvalidIDException e) {
return false;
}
return instances.containsKey(validateId(id));
}

/** {@inheritDoc} */
Expand All @@ -155,28 +140,6 @@ public boolean exists(String id) {
return Collections.unmodifiableMap(this.instances);
}

/** {@inheritDoc} */
@Override
public boolean exists(String id, Class<? extends IO> type){
String _id = null;
try {
_id = validateId(id);

// return 'false' if the requested ID is not found
if(!instances.containsKey(_id))
return false;

// get the I/O instance
IO instance = instances.get(id);

// return true if the I/O instance matches the requested I/O type
return type.isAssignableFrom(instance.getClass());
} catch (IOInvalidIDException e) {
return false;
}
}


private String validateId(String id) throws IOInvalidIDException {
if(id == null)
throw new IOInvalidIDException();
Expand Down Expand Up @@ -205,5 +168,4 @@ public RuntimeRegistry initialize() throws InitializeException {
// NOTHING TO INITIALIZE
return this;
}

}

0 comments on commit 971ad30

Please sign in to comment.