[JCS-12] Non thread safe OOS writes

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: jcs-1.2.7.8, jcs-1.2.7.9
• Fix Version/s: jcs-1.2.7.9.2
• Component/s: TCP Lateral Cache
• Environment: all
• Assignee: Aaron Smuts
• Reporter: Aaron Smuts
• Created: 22/Sep/06 17:28
• Updated: 22/Sep/06 18:32
• Resolved: 22/Sep/06 18:00

Description

Hi,

We encountered a problem with an infinite loop in
java.io.ObjectOutputStream
which we were able to fix by ensuring that access to
it was synchronized
inside LateralTCPSender.java. I thought I should
report exactly what we
found and what we did to solve the problem so that
perhaps similar changes
might be made for the next release of jcs.

We were using the LateralTCPCache in jcs.1.2.7.9 and
were performing
serveral thousand gets and puts per minute from
multiple threads all using
the same JCS object. Very quickly, one of these
threads would end up in an
infinite loop in ObjectOutputStream.lookup()
(offending loop in bold)

int lookup(Object obj) {
if (size == 0)

{ return -1; }

int index = hash(obj) % spine.length;
for (int i = spine[index]; i >= 0; i = next[i]) {
if (objs[i] == obj)

{ return i; }

}
return -1;
}

Once one thread ended up in this loop, all other
threads would end up
blocking, waiting to obtain a lock on this.getLock in
LateralTCPSender.sendAndReceive() (found by performing
a thread dump after
the application had stopped responding). But since
the thread in the
inifinite loop already had a lock on this.getLock,
our application was
effectively dead.

Now ObjectOutputStream is not thread-safe. Indeed, if
multiple threads
attempt to write objects to the same
ObjectOutputStream simultaneously, it
is likely that the internal object cache will become
corrupted and
subsequent writes will end up locked in the
aforementioned infinite loop.

Looking at LateralTCPSender.java more carefully, we
noticed that the send(
LateralElementDescriptor led ) method did perform
non-thread-safe writes to
the shared ObjectOutputStream. Wrapping access to
it with a
synchronized(this.getLock) block (exactly as was done
in the sendAndReceive
method appears to have fixed out problem. I've
included the 1.2.7.9
version of this method with the changes we made in
bold.

/**

• Sends commands to the lateral cache listener.
• <p>
• @param led
• @throws IOException
  */
  public void send( LateralElementDescriptor led )
  throws IOException
  {
  sendCnt++;
  if ( log.isInfoEnabled() )
  Unknown macro: { if ( sendCnt % 100 == 0 ) { log.info( "Send Count (port " + port + ") = " + sendCnt ); } }

if ( log.isDebugEnabled() )

{ log.debug( "sending LateralElementDescriptor" ); }

if ( led == null )

{ return; }

if ( address == null )

{ throw new IOException( "No remote host is set for LateralTCPSender." ); }

if ( oos != null )
{
synchronized(this.getLock)
{ try
{
oos.writeObject( led );
oos.flush();
if ( ++counter >= RESET_FREQUENCY
)
{
counter = 0;
// Failing to reset the object
output stream every
now and
// then creates a serious
memory leak.
if ( log.isDebugEnabled() )

{ log.debug( "Doing oos.reset()" ); }

oos.reset();
}
}
catch ( IOException e )

{ oos = null; log.error( "Detected problem with connection: " + e ); throw e; }

} }
}

Comments?
John (jklame)