NIFI-6341: Addressed bug that can result in a deadlock if Thread 1 adds a Controller Service to a Process Group (which obtains group's write lock and AbstractComponent's Lock) while at the same time Thread 2 attempts to peform validation on the service (which obtains Controller Service's write lock, then AbstractComponent's Lock)

This closes #3510.

Signed-off-by: Bryan Bende <bbende@apache.org>
This commit is contained in:
Mark Payne 2019-05-31 15:50:19 -04:00 committed by Bryan Bende
parent 08d2e69878
commit 60b5c13ce9
No known key found for this signature in database
GPG Key ID: A0DDA9ED50711C39
2 changed files with 27 additions and 47 deletions

View File

@ -144,8 +144,7 @@ public class StandardProcessorNode extends ProcessorNode implements Connectable
private volatile ScheduledState desiredState = ScheduledState.STOPPED;
private volatile LogLevel bulletinLevel = LogLevel.WARN;
private SchedulingStrategy schedulingStrategy; // guarded by read/write lock
// ??????? NOT any more
private SchedulingStrategy schedulingStrategy; // guarded by synchronized keyword
private ExecutionNode executionNode;
private final Map<Thread, ActiveTask> activeThreads = new HashMap<>(48);
private final int hashCode;

View File

@ -16,23 +16,6 @@
*/
package org.apache.nifi.controller.service;
import java.lang.reflect.InvocationTargetException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.apache.commons.lang3.StringUtils;
import org.apache.nifi.annotation.behavior.Restricted;
import org.apache.nifi.annotation.documentation.DeprecationNotice;
@ -68,6 +51,24 @@ import org.apache.nifi.util.file.classloader.ClassLoaderUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.lang.reflect.InvocationTargetException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
public class StandardControllerServiceNode extends AbstractComponentNode implements ControllerServiceNode {
private static final Logger LOG = LoggerFactory.getLogger(StandardControllerServiceNode.class);
@ -82,8 +83,8 @@ public class StandardControllerServiceNode extends AbstractComponentNode impleme
private final Lock writeLock = rwLock.writeLock();
private final Set<ComponentNode> referencingComponents = new HashSet<>();
private String comment;
private ProcessGroup processGroup;
private volatile String comment;
private volatile ProcessGroup processGroup;
private final AtomicBoolean active;
@ -207,24 +208,14 @@ public class StandardControllerServiceNode extends AbstractComponentNode impleme
@Override
public ProcessGroup getProcessGroup() {
readLock.lock();
try {
return processGroup;
} finally {
readLock.unlock();
}
return processGroup;
}
@Override
public void setProcessGroup(final ProcessGroup group) {
writeLock.lock();
try {
this.processGroup = group;
LOG.debug("Resetting Validation State of {} due to setting process group", this);
resetValidationState();
} finally {
writeLock.unlock();
}
this.processGroup = group;
LOG.debug("Resetting Validation State of {} due to setting process group", this);
resetValidationState();
}
@Override
@ -339,22 +330,12 @@ public class StandardControllerServiceNode extends AbstractComponentNode impleme
@Override
public String getComments() {
readLock.lock();
try {
return comment;
} finally {
readLock.unlock();
}
return comment;
}
@Override
public void setComments(final String comment) {
writeLock.lock();
try {
this.comment = CharacterFilterUtils.filterInvalidXmlCharacters(comment);
} finally {
writeLock.unlock();
}
this.comment = CharacterFilterUtils.filterInvalidXmlCharacters(comment);
}
@Override