[jboss-jira] [JBoss JIRA] Commented: (JBCACHE-938) OptimisticNodeInterceptor may NPE if node cannot be created

[Manik Surtani (JIRA)]

    [ http://jira.jboss.com/jira/browse/JBCACHE-938?page=comments#action_12351018 ] 
            
Manik Surtani commented on JBCACHE-938:
---------------------------------------

The section in [1] only makes sense/would be triggered in event of a node or data removal.  Not much else.  (Put methods will have gone through the OptimisticCreateIfNotExistsInterceptor and a node would be created.

As such, the null check you see above is probably unnecessary.

Do you have a test that causes this to trip up?

> OptimisticNodeInterceptor may NPE if node cannot be created
> -----------------------------------------------------------
>
>                 Key: JBCACHE-938
>                 URL: http://jira.jboss.com/jira/browse/JBCACHE-938
>             Project: JBoss Cache
>          Issue Type: Bug
>      Security Level: Public(Everyone can see) 
>    Affects Versions: 2.0.0.ALPHA2
>            Reporter: Elias Ross
>         Assigned To: Manik Surtani
>             Fix For: 2.0.0.BETA2
>
>
> ...
>          else
>          {
>             // "fail-more-silently" patch thanks to Owen Taylor - JBCACHE-767
>             if ((ctx.getOptionOverrides() == null || !ctx.getOptionOverrides().isFailSilently()) && MethodDeclarations.isPutMethod(m.getMethodId()))
>             {
>                throw new CacheException("Unable to set node version for " + getFqn(args) + ", node is null.");
>             }
>       /* [1] */
>          }
>          switch (m.getMethodId())
>          {
>             case MethodDeclarations.putDataMethodLocal_id:
>                Boolean erase = (Boolean) args[3];
>                cache.getNotifier().notifyNodeModified(fqn, true, CacheListener.ModificationType.PUT_MAP, workspaceNode == null ? null : workspaceNode.getData() , false);
>                putDataMap((Map<Object, Object>) args[2], erase, workspace, workspaceNode);
>                cache.getNotifier().notifyNodeModified(fqn, false, CacheListener.ModificationType.PUT_MAP, workspaceNode.getData(), false);
> // ^^^ NPE on workspaceNode.getData()
>                break;
> ...
> If the node fails to be created or located, the "workspaceNode" variable may be null.
> The first "? :" check checks for a null condition.
> The second does not.
> I don't believe notification should take place if there is no workspaceNode to modify.
> The solution might be to have notifyNodeModified() take a Node and not a Map. Then, this method code check for the null case.
> Though, I think in general, it's bad to have the code try and handle this edge case, as most maintainers might not expect workspaceNode to be null during the manipulation steps. Perhaps an ELSE statement could be put in [1] to abort this method gracefully?

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira