transporterclient not thread-safe for failover ---------------------------------------------- Key: JBREM-857 URL: https://issues.jboss.org/browse/JBREM-857 Project: JBoss Remoting Issue Type: Bug Security Level: Public(Everyone can see) Components: transporter Affects Versions: 1.4.6.GA, 2.2.2.SP2, 2.4.0.Beta1 (Pinto) Environment: solaris 10 and java 5 Reporter: craig bordelon Fix For: 2.5.3.Beta (Flounder) Original Estimate: 1 day Remaining Estimate: 1 day the jboss 1.4.1 transporterclient used by multithreaded program was giving strange results on a clustered server when failing over so i found lack of synchronization in the code. I made these changes and seems to be helping. changes to TransporterClient.java. I would like this fixed in the 2.x and 3.x versions (hello developers). I think i know how to fix the 2.x version of this code too as it has some new stuff but is likewise broken. (developers send me an email to bord(a)iscp.telcordia.com if you need it. thx Craig) @@ -93,11 +93,12 @@ /** * Disconnects the remoting client */ - private void disconnect() + private synchronized void disconnect() { if(remotingClient != null) { remotingClient.disconnect(); + remotingClient = null; } } @@ -232,14 +233,18 @@ do { + Client rcl = null; + synchronized(this) { rcl = remotingClient; } + if (rcl == null) throw new CannotConnectException("Client has been dis connected."); try { failOver = false; - response = remotingClient.invoke(request); + response = rcl.invoke(request); } catch(CannotConnectException cnc) { - failOver = findAlternativeTarget(); + failOver = findAlternativeTarget(rcl); + if (log.isDebugEnabled()) log.error("on CannotConnectException failO ver is "+failOver+" for "+rcl.getInvoker().getLocator()); if(!failOver) { throw cnc; @@ -264,7 +269,7 @@ * * @return */ - private boolean findAlternativeTarget() + private boolean findAlternativeTarget(Client rcl) { boolean failover = false; @@ -287,12 +292,19 @@ { // finally found server with target handler InvokerLocator newLocator = data.getInvokerLocator(); - if(!remotingClient.getInvoker().getLocator().equals(new Locator)) + if(!rcl.getInvoker().getLocator().equals(newLocator)) { try { - remotingClient = new Client(newLocator); - remotingClient.connect(); + synchronized(this) { + if (rcl == remotingClient) { + disconnect(); + remotingClient = new Client(newLocator); + } + //otherwise have to assume some other thread + //has meanwhile changed it + //including even disconnecting + } return true; } catch(Exception e) @@ -300,6 +312,8 @@ log.warn("Problem connecting to newly found alter nate target.", e); } } + else if (log.isDebugEnabled()) + log.error(rcl.getInvoker().getLocator()+" == "+newLoc ator); } } } @@ -333,4 +347,4 @@ }