#8 Race condition

[David Hoag]

There is a potential "race condition" within JGrinder. This
problem can cause the uninitialized state of an object to
be exposed, potentially resulting in random application
errors.

In the current implementation of JGrinder, an object is
placed into the object pool prior to the initialization of
that object's state. While one thread (T1) is creating
and initializing the persistent object, other threads (Tn)
may find this object in the pool and access its
uninitialized state prior to the completion of its
initialization. The "window of opportunity" for such
problems is small, and it is likely that actual problems will
be found only in very busy environments that have
threads contending for the same objects. Problems that
occur are likely to be intermittent, and may occur only
on startup (when objects are being cached in the pool).

The problem was traced to the following method in
ProcessResultSet:

public Persistence buildObject( final
RDBPersistence refObj,
RDBPersistence origObj, final RDBPersistence origRef,
Object [] dataRow)
throws InstantiationException, IllegalAccessException
{
Persistence obj;

if (origObj instanceof
RDBExtendablePersistentAdapter)
{
// [code omitted for clarity]
}
else
{
obj = createNewObject
(refObj, origObj, origRef,
dataRow);
}
obj.setRetrievedFromDatabase(true);
obj.setBrokerName
(origObj.getBrokerName());

Persistence pooledObject =
checkObjectPool(obj, refObj,
origObj, origRef, dataRow);
if(pooledObject != obj)
{
return pooledObject;
}
if(verbose)
{
println("Building object " +
obj.getClass().getName() + '@' + obj.hashCode());
}
boolean valid = buildAttributes(obj,
refObj, origObj,
origRef, dataRow);
if(valid) //A Valid object with a non
zero primary key
field.
{
buildForeignKeyObjects(obj, refObj,
origObj,
origRef, dataRow);
buildInstanceLinkObjects
(obj, refObj, origObj,
origRef, dataRow);
buildCollectionProxies(obj,
refObj, origObj,
origRef);
}
else
{
BrokerFactory.println
("Rejecting retrieved object
due to bad primary key value.");
obj = null;
}
return obj;
}

It is the call to checkObjectPool() that is the problem.
This method will check the pool for an instance with the
specified primary key. If one exists, it will be returned
(and no problems will be encountered). However, if no
instance with the specified primary key exists, the newly
created instance will be added to the pool. At this point,
the instance's primary key is initialized, but none of the
attributes or relationships have been initialized. Other
threads that access this object from the pool may see
the instance's uninitialized state.

A test case was created to validate this problem. The
test case involved two threads... each thread trying
to "realize" the same instance in different ways. The first
thread was loading the object by realizing a proxy. The
second thread used a SQLQuery. The first thread was
allowed to proceed, but was suspended immediately
after the call to checkObjectPool() using a debugger
breakpoint. The breakpoint served as a useful simulator
of a "thread context switch" in the VM. The second
thread was then triggered, executed its query by
primary key (which found the uninitialized instance in
the pool), and then displayed the object's attributes in a
loop. After allowing the first thread to continue, the
values displayed by the second thread changed from
uninitialized (in this case, null) to the correct
database values.

A potentional fix to resolve this problem, move the
checkObjectPool() call to after the object's state has
been initialized. The new code would be:

public Persistence buildObject( final
RDBPersistence refObj,
RDBPersistence origObj, final RDBPersistence origRef,
Object [] dataRow)
throws InstantiationException, IllegalAccessException
{
Persistence obj;

if (origObj instanceof
RDBExtendablePersistentAdapter)
{
// [code omitted for clarity]
}
else
{
obj = createNewObject
(refObj, origObj, origRef,
dataRow);
}
obj.setRetrievedFromDatabase(true);
obj.setBrokerName
(origObj.getBrokerName());

// moved pooled object check to the
end

if(verbose)
{
println("Building object " +
obj.getClass().getName() + '@' + obj.hashCode());
}
boolean valid = buildAttributes(obj,
refObj, origObj,
origRef, dataRow);
if(valid) //A Valid object with a non
zero primary key
field.
{
buildForeignKeyObjects(obj,
refObj, origObj,
origRef, dataRow);
buildInstanceLinkObjects
(obj, refObj, origObj,
origRef, dataRow);
buildCollectionProxies(obj,
refObj, origObj,
origRef);
}
else
{
BrokerFactory.println
("Rejecting retrieved object
due to bad primary key value.");
obj = null;
}

// check with the object pool -- if
already in the pool,
return pooled object;
// if not yet in pool, will insert obj
into pool and return
same
return checkObjectPool(obj, refObj,
origObj, origRef,
dataRow);
}

There are cases where buildObject() is invoked
recursively, and there was some initial concern that by
moving this code, we might introduce an infinite
recursion. After some testing and thought, though, I
think this is not a likely outcome. In the extreme case
(with join'ed queries on data with looping relationships --
e.g., A refers to B, B refers to A), the worst I can see is
that extra instances will be created that will be thrown
away.