Tomas Jelinek has uploaded a new change for review. Change subject: engine: fix race in CommandAsyncTask ......................................................................
engine: fix race in CommandAsyncTask The race was the following: 1: [thread 1] CommandAsyncTask's constructor reaches the point "entityInfo.AttachTask(this)" while the "_multiTasksByCommandIds" still contains one task. After this it gets interrupted 2: [thread 2] the CommandAsyncTask.handleEndActionResult is executed in meanwhile and issues the "_multiTasksByCommandIds.remove(commandInfo.getCommandId());" 3: [thread 1] the first thread continues the execution and calls the "entityInfo.AttachTask(this);" 4: [thread 1] the CommandAsyncTask.ConcreteStartPollingTask() is called. It asks for the GetCommandMultiAsyncTasks which returns null (because thread 2 have deleted it) and fails on NPE on the next line Fixed by moving all the code which needs to be synchonized to the sync block. It will not cause any deadlocks because the CommandMultiAsyncTasks.AttachTask does not use the original _lockObject (nor directly nor indirectly). Change-Id: Id265df1c1843b976c6f8d65729fd6717c3df3c76 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1015638 Signed-off-by: Tomas Jelinek <tjeli...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java 1 file changed, 8 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/45/20845/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java index 6b1a5b3..8ac3f9e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java @@ -46,7 +46,11 @@ _multiTasksByCommandIds.put(getCommandId(), new CommandMultiAsyncTasks(getCommandId())); isNewCommandAdded = true; } + + CommandMultiAsyncTasks entityInfo = GetCommandMultiAsyncTasks(); + entityInfo.AttachTask(this); } + if (duringInit && isNewCommandAdded) { CommandBase<?> command = CommandsFactory.CreateCommand(parameters.getDbAsyncTask().getaction_type(), @@ -58,8 +62,7 @@ } } - CommandMultiAsyncTasks entityInfo = GetCommandMultiAsyncTasks(); - entityInfo.AttachTask(this); + } @Override @@ -198,7 +201,9 @@ log.infoFormat( "CommandAsyncTask::HandleEndActionRuntimeException: Removing CommandMultiAsyncTasks object for entity '{0}'", commandInfo.getCommandId()); - _multiTasksByCommandIds.remove(commandInfo.getCommandId()); + synchronized (_lockObject) { + _multiTasksByCommandIds.remove(commandInfo.getCommandId()); + } } } -- To view, visit http://gerrit.ovirt.org/20845 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id265df1c1843b976c6f8d65729fd6717c3df3c76 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.3.1 Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
