cloudstack-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CLOUDSTACK-9796) Null Pointer Exception in VirtualMachineManagerImpl.java
Date Tue, 21 Feb 2017 19:48:44 GMT

    [ 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)

Mime
View raw message