[
https://issues.apache.org/jira/browse/CLOUDSTACK-9796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876582#comment-15876582
]
ASF GitHub Bot commented on CLOUDSTACK-9796:
--------------------------------------------
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1956#discussion_r102300083
--- Diff:
engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java ---
@@ -744,14 +744,17 @@ protected boolean checkWorkItems(final VMInstanceVO
vm, final State state) throw
protected <T extends VMInstanceVO> boolean changeState(final T vm,
final Event event, final Long hostId, final ItWorkVO work, final Step step)
throws NoTransitionException {
// FIXME: We should do this better.
- final Step previousStep = work.getStep();
- _workDao.updateStep(work, step);
+ Step previousStep = null;
+ if (work != null) {
+ previousStep = work.getStep();
--- End diff --
I know you did not write this code, but it seemed a good opportunity to
discuss and evaluate it.
I understood you. I already noticed the try/finally block, and this is the
point I wanted to discuss. As the example you described, if an exception
happens, the finally block is executed and the state is restored to a previous
one (assuming that the `stateTransitTo(vm, event, hostId)` will change the
step); and this makes sense in the case of an exception. However, if `NO`
exception happens, the step is also reverted to a previous one (assuming that
the `stateTransitTo(vm, event, hostId)` will change the step) . The `finally `
is always executed; either with successful or unsuccessful execution of
`stateTransitTo(vm, event, hostId)`.
If we wanted to deal with exceptions, it would make much more sense
executing the revert on a `catch` block. I think that we want/need to change
the step for `null` when `stateTransitTo` return false and the `previousStep`
is null . You are changing exactly that with the extra condition at line 757.
Did you understand what I mean?
> Null Pointer Exception in VirtualMachineManagerImpl.java
> --------------------------------------------------------
>
> Key: CLOUDSTACK-9796
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9796
> Project: CloudStack
> Issue Type: Bug
> Security Level: Public(Anyone can view this level - this is the
> default.)
> Affects Versions: 4.8.0, 4.9.0
> Environment: Cloudstack 4.8
> Reporter: Nathan Johnson
> Assignee: Nathan Johnson
> Priority: Minor
> Attachments: npelog.txt
>
>
> When a situation occurs where a VM hangs in the "Starting" state for longer
> than the job.expire.minutes, and the job is deleted from the system, a null
> pointer exception will occur because the work VO will be null inside of
> advanceStop in VirtualMachineManagerImpl.java . I have attached a snippet of
> a log file of this NPE occurring in the wild.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
