Author: jiwils Date: 2009-04-01 23:34:16 -0400 (Wed, 01 Apr 2009) New Revision: 7959 Modified: core/branches/1.4.X/src/org/jboss/cache/TreeCache.java core/branches/1.4.X/tests/functional/org/jboss/cache/TreeCacheFunctionalTest.java Log: Fix for JBCACHE-1483. findNode didn't correctly handle situations when optimistic locking was used and findInternal returned null. Modified: core/branches/1.4.X/src/org/jboss/cache/TreeCache.java =================================================================== --- core/branches/1.4.X/src/org/jboss/cache/TreeCache.java 2009-03-31 05:46:19 UTC (rev 7958) +++ core/branches/1.4.X/src/org/jboss/cache/TreeCache.java 2009-04-02 03:34:16 UTC (rev 7959) @@ -113,8 +113,8 @@ private static final String JGROUPS_JMX_DOMAIN = "jboss.jgroups"; private static final String CHANNEL_JMX_ATTRIBUTES = "type=channel,cluster="; private static final String PROTOCOL_JMX_ATTRIBUTES = "type=protocol,cluster="; - - + + private boolean forceAnycast = false; /** * Default replication version, from {@link Version#getVersionShort}. @@ -285,7 +285,7 @@ * Require write locks on parents before adding or removing children. Default false. */ protected boolean lockParentForChildInsertRemove = false; - + /** * This ThreadLocal contains an {@see InvocationContext} object, which holds * invocation-specific details. @@ -436,15 +436,15 @@ * will be ignored by _getState(). */ protected final Set activationChangeNodes = new HashSet(); - + /** - * The JGroups 2.4.1 or higher "callRemoteMethods" overload that + * The JGroups 2.4.1 or higher "callRemoteMethods" overload that * provides Anycast support. */ protected Method anycastMethod; - + /** Did we register our channel in JMX ourself? */ - protected boolean channelRegistered; + protected boolean channelRegistered; /** Did we register our channel's protocols in JMX ourself? */ protected boolean protocolsRegistered; @@ -1126,7 +1126,7 @@ public IsolationLevel getIsolationLevelClass() { return isolationLevel; - } + } /** * Gets whether inserting or removing a node requires a write lock @@ -1475,7 +1475,7 @@ channel = new JChannel(cluster_props); // JBCACHE-1048 Hack to register the channel registerChannelInJmx(); - + // Only set GET_STATE_EVENTS if the JGroups version is < 2.3 int jgVer= org.jgroups.Version.version; while (jgVer >= 100) @@ -1498,7 +1498,7 @@ */ disp = new RpcDispatcher(channel, ml, this, this); disp.setMarshaller(getMarshaller()); - + // See if Anycast is supported try { @@ -1506,7 +1506,7 @@ } catch (Throwable ignored) { - log.debug("JGroups release " + org.jgroups.Version.version + + log.debug("JGroups release " + org.jgroups.Version.version + " does not support anycast; will not use it"); } break; @@ -1990,13 +1990,13 @@ { // Get the state from each DataOwner and integrate in their // respective buddy backup tree - List buddies = buddyManager.getBackupDataOwners(); + List buddies = buddyManager.getBackupDataOwners(); for (Iterator it = buddies.iterator(); it.hasNext();) { Address buddy = (Address) it.next(); if (getMembers() == null || !getMembers().contains(buddy)) continue; - + Object[] sources = {buddy}; Fqn base = new Fqn(BuddyManager.BUDDY_BACKUP_SUBTREE_FQN, BuddyManager.getGroupNameFromAddress(buddy)); Fqn buddyRoot = new Fqn(base, fqn); @@ -2302,7 +2302,7 @@ child = factory.createDataNode(type, name, subtree.getFqnChild(i + 1), parent, null, this); - // Add uninitialized flag so that data stored at the root of the + // Add uninitialized flag so that data stored at the root of the // region can be loaded/preloaded from the cache loader. child.put(UNINITIALIZED, null); parent.addChild(name, child); @@ -3366,7 +3366,7 @@ // No one provided us with state. We need to find out if that's because // we are the coordinator. But we don't know if the viewAccepted() callback // has been invoked, so call determineCoordinator(), which will block until - // viewAccepted() is called at least once + // viewAccepted() is called at least once determineCoordinator(); if (isCoordinator()) @@ -5088,16 +5088,16 @@ * Internal evict method called by eviction policy provider. * * @param fqn removes everything assoicated with this FQN - * - * @return <code>true</code> if the node has been completely removed, + * + * @return <code>true</code> if the node has been completely removed, * <code>false</code> if only the data map was removed, due * to the presence of children - * + * * @throws CacheException */ public boolean _evict(Fqn fqn) throws CacheException { - if (!exists(fqn)) + if (!exists(fqn)) return true; // node does not exist. Maybe it has been recursively removed. // use remove method now if there is a child node. Otherwise, it is removed boolean create_undo_ops = false; @@ -5122,16 +5122,16 @@ * * @param fqn * @param version - * - * @return <code>true</code> if the node has been completely removed, + * + * @return <code>true</code> if the node has been completely removed, * <code>false</code> if only the data map was removed, due * to the presence of children - * + * * @throws CacheException */ public boolean _evict(Fqn fqn, DataVersion version) throws CacheException { - if (!exists(fqn)) + if (!exists(fqn)) return true; // node does not exist boolean create_undo_ops = false; @@ -5762,7 +5762,7 @@ // Now that we have a view, figure out if we are the coordinator coordinator = (members.size() != 0 && members.get(0).equals(getLocalAddress())); - // now notify listeners - *after* updating the coordinator. - JBCACHE-662 + // now notify listeners - *after* updating the coordinator. - JBCACHE-662 if (needNotification) notifyViewChange(new_view); // Wake up any threads that are waiting to know who the members @@ -5869,7 +5869,7 @@ catch (SystemException e) { } - + // JBCACHE-982 -- don't complain if COMMITTED if (status != Status.STATUS_COMMITTED) { @@ -5879,7 +5879,7 @@ { log.trace("status is COMMITTED; returning null"); } - + return null; } @@ -6016,7 +6016,7 @@ Node toReturn = findInternal(fqn, false); - if (version != null) + if (toReturn != null && version != null) { // we need to check the version of the data node... DataVersion nodeVersion = ((OptimisticTreeNode) toReturn).getVersion(); @@ -6311,7 +6311,7 @@ if (curr == null) return; notifyNodeCreated(curr.getFqn()); - + if ((children = curr.getChildren()) != null) { for (Iterator it = children.values().iterator(); it.hasNext();) @@ -6719,7 +6719,7 @@ return (MBeanServer) servers.get(0); } - + protected void registerChannelInJmx() { if (server != null) @@ -6729,7 +6729,7 @@ String protocolPrefix = JGROUPS_JMX_DOMAIN + ":" + PROTOCOL_JMX_ATTRIBUTES + getClusterName(); JmxConfigurator.registerProtocols(server, channel, protocolPrefix); protocolsRegistered = true; - + String name = JGROUPS_JMX_DOMAIN + ":" + CHANNEL_JMX_ATTRIBUTES + getClusterName(); JmxConfigurator.registerChannel(channel, server, name); channelRegistered = true; @@ -6740,12 +6740,12 @@ } } } - + protected void unregisterChannelFromJmx() { ObjectName on = null; if (channelRegistered) - { + { // Unregister the channel itself try { @@ -6760,7 +6760,7 @@ log.error("Caught exception unregistering channel", e); } } - + if (protocolsRegistered) { // Unregister the protocols Modified: core/branches/1.4.X/tests/functional/org/jboss/cache/TreeCacheFunctionalTest.java =================================================================== --- core/branches/1.4.X/tests/functional/org/jboss/cache/TreeCacheFunctionalTest.java 2009-03-31 05:46:19 UTC (rev 7958) +++ core/branches/1.4.X/tests/functional/org/jboss/cache/TreeCacheFunctionalTest.java 2009-04-02 03:34:16 UTC (rev 7959) @@ -160,8 +160,18 @@ return tmptx; } + public void testFindNodeWhenFindInternalReturnsNull() throws CacheException { + try { + // This remove causes findNode to be invoked and findInternal to return null + // which used to throw an NPE since findNode didn't handle the situation. See + // JBCACHE-1483 for more information. + cache._remove(null, FQN, false, false, true, new org.jboss.cache.optimistic.DefaultDataVersion()); + } + catch (NullPointerException npe) { + fail("cache.findNode threw an NPE when cache.findInternal returned null."); + } + } - public static Test suite() { return new TestSuite(TreeCacheFunctionalTest.class); }