transporterclient not thread-safe for failover

[bord]

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 [e-mail] 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 @@
}

[bord]

Continuing on this suject...
As i just learned about jira issue tracking -- and very hard to find my way around the site to issue a bug to jboss (as opposed to jira itself), I have now entered a bug on this topic.
Craig

[ron_sigal]

Ok, I see JBREM-857 "transporterclient not thread-safe for failover". Thanks, Craig. Depending on timing, the fix may or may not make into the next release.