You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by bb...@apache.org on 2019/06/04 13:55:52 UTC
[nifi] branch master updated: 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 is an automated email from the ASF dual-hosted git repository.
bbende pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nifi.git
The following commit(s) were added to refs/heads/master by this push:
new 60b5c13 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)
60b5c13 is described below
commit 60b5c13ce95fd4d4a5edf0f08b81af19e71b67ee
Author: Mark Payne <ma...@hotmail.com>
AuthorDate: Fri May 31 15:50:19 2019 -0400
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 <bb...@apache.org>
---
.../nifi/controller/StandardProcessorNode.java | 3 +-
.../service/StandardControllerServiceNode.java | 71 ++++++++--------------
2 files changed, 27 insertions(+), 47 deletions(-)
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardProcessorNode.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardProcessorNode.java
index 3e407f5..f8cbd61 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardProcessorNode.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardProcessorNode.java
@@ -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;
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceNode.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceNode.java
index 6579480..aae22ab 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceNode.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceNode.java
@@ -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