Skip to content

Commit 53ebf50

Browse files
committed
cells: Let CellPath#revert return a copy
CellPath#revert reverts the path inline. In almost all cases this means one should clone the CellPath object before reverting it. This patch updates revert to return a reverted copy of the path rather than modifying the path itself. This protects against accidentally modifying shared cell path. The patch also modifies CellMessage to clone the injected CellPath. This too is to avoid that shared CellPath objects get modified. Targe: trunk Require-book: no Require-notes: no Acked-by: Paul Millar <paul.millar@desy.de> Patch: http://rb.dcache.org/r/5393/
1 parent edd2b6e commit 53ebf50

File tree

12 files changed

+23
-38
lines changed

12 files changed

+23
-38
lines changed

modules/cells/src/main/java/dmg/cells/network/LocationMgrTunnel.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,7 @@ private void returnToSender(CellMessage msg, NoRouteToCellException e)
133133
{
134134
try {
135135
if (!(msg instanceof CellExceptionMessage)) {
136-
CellPath retAddr = (CellPath)msg.getSourcePath().clone();
137-
retAddr.revert();
136+
CellPath retAddr = msg.getSourcePath().revert();
138137
CellExceptionMessage ret = new CellExceptionMessage(retAddr, e);
139138
ret.setLastUOID(msg.getUOID());
140139
_nucleus.sendMessage(ret);

modules/cells/src/main/java/dmg/cells/nucleus/CellGlue.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -676,8 +676,7 @@ private void sendException( CellNucleus nucleus ,
676676
destination ,
677677
"Tunnel cell >"+routeTarget+
678678
"< not found at >"+_cellDomainName+"<" ) ;
679-
CellPath retAddr = (CellPath)msg.getSourcePath().clone() ;
680-
retAddr.revert() ;
679+
CellPath retAddr = msg.getSourcePath().revert();
681680
CellExceptionMessage ret =
682681
new CellExceptionMessage( retAddr , exception ) ;
683682
LOGGER.warn("Sending CellException to {}", retAddr);

modules/cells/src/main/java/dmg/cells/nucleus/CellMessage.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public class CellMessage implements Cloneable , Serializable {
3737

3838
public CellMessage( CellPath addr , Serializable msg ){
3939

40-
_destination = addr ;
40+
_destination = (CellPath) addr.clone();
4141
_message = msg ;
4242
_source = new CellPath() ;
4343
_creationTime = System.currentTimeMillis() ;
@@ -102,8 +102,7 @@ public void setLastUOID( UOID lastUOID ) {
102102
public Serializable getMessageObject(){ return (Serializable) _message ; }
103103
public void setMessageObject( Serializable obj ){ _message = obj ; }
104104
public void revertDirection(){
105-
_destination = _source ;
106-
_destination.revert() ;
105+
_destination = _source.revert();
107106
_source = new CellPath() ;
108107
_lastUmid = _umid ;
109108
_isPersistent = true;

modules/cells/src/main/java/dmg/cells/nucleus/CellPath.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package dmg.cells.nucleus ;
22

3+
import com.google.common.collect.Lists;
4+
35
import java.io.Serializable;
46
import java.util.ArrayList;
57
import java.util.Collections;
@@ -138,14 +140,13 @@ public synchronized boolean next(){
138140
_position++ ;
139141
return true;
140142
}
141-
public synchronized void toFirstDestination(){
142-
_position = _list.size() == 0 ? -1 : 0 ;
143-
}
144-
public synchronized void revert(){
145143

146-
Collections.reverse(_list);
147-
toFirstDestination() ;
148-
}
144+
public synchronized CellPath revert(){
145+
CellPath copy = new CellPath();
146+
copy._list = Lists.newArrayList(Lists.reverse(_list));
147+
copy._position = copy._list.size() == 0 ? -1 : 0 ;
148+
return copy;
149+
}
149150
public synchronized boolean isFinalDestination(){
150151
return _position >= ( _list.size() - 1 ) ;
151152
}

modules/cells/src/main/java/dmg/cells/services/multicaster/BroadcastCell.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,7 @@ private void handleBroadcastCommandMessage( CellMessage msg , BroadcastCommandMe
349349
String eventClass = event.getEventClass() ;
350350
CellPath target = event.getTarget() ;
351351
if( target == null ){
352-
target = (CellPath)msg.getSourcePath().clone() ;
353-
target.revert() ;
352+
target = msg.getSourcePath().revert();
354353
}
355354
if( event instanceof BroadcastRegisterMessage ){
356355
BroadcastRegisterMessage reg = (BroadcastRegisterMessage)event ;

modules/cells/src/main/java/dmg/cells/services/multicaster/MulticastCell.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,7 @@ private void registerArrived( MulticastRegister register , CellPath path )
219219
throw new
220220
NoSuchElementException("Not found : " + eventClass + ":" + eventName);
221221
}
222-
path.revert() ;
223-
entry.addClient(new Client(path)) ;
222+
entry.addClient(new Client(path.revert())) ;
224223
register.setServerInfo( entry.getServerDetail() , entry.getServerState() ) ;
225224
}
226225
private void unregisterArrived( MulticastUnregister register , CellPath path )
@@ -231,8 +230,7 @@ private void unregisterArrived( MulticastUnregister register , CellPath path )
231230
if( entry == null ) {
232231
return;
233232
}
234-
path.revert() ;
235-
entry.removeClient(path) ;
233+
entry.removeClient(path.revert()) ;
236234
}
237235
private void registerMessageArrived(
238236
MulticastMessage message ,
@@ -271,8 +269,7 @@ private void registerMessageArrived(
271269
}
272270
}else{
273271
_log.info( "Message from client "+path ) ;
274-
serverPath = (CellPath)serverPath.clone() ;
275-
serverPath.revert() ;
272+
serverPath = serverPath.revert();
276273
originalMessage.getDestinationPath().add( serverPath ) ;
277274
originalMessage.nextDestination() ;
278275
try{

modules/cells/src/main/java/dmg/cells/services/multicaster/MulticastCommander.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,8 @@ public void messageToForward( CellMessage msg ){
3737
}catch( NoRouteToCellException nrtc ){
3838
_log.warn( "NoRouteToCell in messageToForward : "+nrtc ) ;
3939
_log.warn( "Sending NoRouteToCellt to : "+source ) ;
40-
source.revert() ;
4140
try{
42-
sendMessage( new CellMessage( source , nrtc ) ) ;
41+
sendMessage( new CellMessage( source.revert() , nrtc ) ) ;
4342
}catch(Exception ee ){
4443
_log.warn( "can't return NoRouteToCell to : "+source ) ;
4544
}

modules/dcache-jms/src/main/java/org/dcache/cells/JMSTunnel.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,7 @@ private void returnToSender(CellMessage msg, NoRouteToCellException e)
240240
if (!(msg instanceof CellExceptionMessage)) {
241241
_log.info("Cannot deliver {}", msg);
242242

243-
CellPath retAddr = (CellPath)msg.getSourcePath().clone();
244-
retAddr.revert();
243+
CellPath retAddr = msg.getSourcePath().revert();
245244
CellExceptionMessage ret = new CellExceptionMessage(retAddr, e);
246245
ret.setLastUOID(msg.getUOID());
247246
sendMessage(ret);

modules/dcache/src/main/java/diskCacheV111/namespace/PnfsManagerV3.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,8 +1420,7 @@ private void listDirectory(CellMessage envelope, PnfsListDirectoryMessage msg)
14201420
(delay == Long.MAX_VALUE)
14211421
? Long.MAX_VALUE
14221422
: delay - envelope.getLocalAge();
1423-
CellPath source = (CellPath)envelope.getSourcePath().clone();
1424-
source.revert();
1423+
CellPath source = envelope.getSourcePath().revert();
14251424
ListHandler handler =
14261425
new ListHandlerImpl(source, envelope.getUOID(),
14271426
msg, initialDelay, delay);

modules/dcache/src/main/java/org/dcache/pool/classic/PoolV4.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -706,21 +706,18 @@ private void ioFile(CellMessage envelope, PoolIoFileMessage message)
706706
String initiator = message.getInitiator();
707707
String pool = message.getPoolName();
708708
String queueName = message.getIoQueueName();
709-
CellPath source = (CellPath)envelope.getSourcePath().clone();
709+
CellPath source = envelope.getSourcePath().revert();
710710
Set<Repository.OpenFlags> openFlags =
711711
(Set<Repository.OpenFlags>) (
712712
message.isPool2Pool() ? Collections.emptySet() :
713713
Collections.singleton(Repository.OpenFlags.NOATIME)
714714
);
715715

716-
String door =
717-
source.getCellName() + "@" + source.getCellDomainName();
718-
719716
/* Eliminate duplicate requests.
720717
*/
721718
if (!(message instanceof PoolAcceptFileMessage)
722719
&& !message.isPool2Pool()) {
723-
720+
String door = source.getDestinationAddress().toString();
724721
JobInfo job = _ioQueue.findJob(door, id);
725722
if (job != null) {
726723
switch (_dupRequest) {
@@ -752,8 +749,6 @@ private void ioFile(CellMessage envelope, PoolIoFileMessage message)
752749
pi);
753750
}
754751

755-
source.revert();
756-
757752
String protocolName = protocolNameOf(pi);
758753
MoverExecutorService moverExecutorService = _moverExecutorServices.getExecutorService(protocolName);
759754
PostTransferExecutionService postExecutorService =

0 commit comments

Comments
 (0)