Re: [OJB-developers] Serious threading/lock problem

[Thomas M. <tho...@ho...>]

Hi Georg,

Georg Schneider wrote:

> Hi,
> 
> I am experiencing a very serious locking problem, and I have expanded
> quite a bit of effort to look into it, I haven't solved it yet, but I have
> found a few interesting things, which I would like to discuss:
> 
> 1.) How do you make sure that 2 threads are synchronized when locking?
> 	e.g. I am using repeatable-read strategy and in
> 	RepeatableReadStragegy.java:
> 
> public boolean readLock(TransactionImpl tx, Object obj)
>     {
> 
>         LockEntry writer = getWriter(obj);
> 
> ====> race condition, another thread could put a writer
> 
>         if (writer == null)
>         {
> 
> -------------------------------------------------------
> 
> maybe synchronization happens somewhere else, but to make sure, I put
> synchronized to all the methods in LockManagerDefaultImpl.java
> 


This strange thing has been "detected" by others before and there have 
been some discussions on this topic.

Here is the management summary:
The OJB Lockmanagement does not rely on Java Locking mechanisms!
The reason. Java locks only works within a single JVM.
But OJB Lockmanagement must properly work accross multiple JVMs.
Thus relying on Java's "synchronized" could result in severe data 
corruption.
The C/S mode PersistentLockMapImpl is backed by a database table that 
may be accessed from any number of clients residing in different JVMs.

The SingleVM mode InMemoryLockMapImpl is meant for use for singlevm mode 
only! But it also does not rely on "synchronized" & co.

The double lock checking is done in the Methods of the LockMapImpl 
classes (getWriter() & co.).



> 2.) Let me explain what problem I am experiencing:
> 	I have an object A which references an Object B (1:1),
> 
> 	Thread-1			Thread-2
> 
> 	oqlquery for A
> 
> 	tx.lock(A,tx.upgrade)
> 
> 					oqlquery for A
> 					(LockNotGrantedException)
> 					(since I use RepeatableRead
> 					this should happen)
> 	A.setFieldB(B)
> 
> 	tx.commit()
> 
> 
> 	oqlquery for A
> 
> 	!!!!!FieldB is null
> 	(should be B)
> 
> 
> 
> I have to mention that Object A is a dynamic Proxy.


Mh. This may be somewhat problematic in your scenario. Using (dynamic) 
Proxies in OJB/ODMG uses a "lazy locking". That is the lock is not 
placed immediately but only when the proxy materializes the "real" 
object. Please try your scneario without dynamic proxies.

[side node on (dynamic) class proxies:
The <class.proxy> concept is not optimal.
Jakob contributed a much better proxy concept for Reference- and 
Collection-attributes.
These concepts are not documented in the current release, but we will 
provide documentation on this feature in the next release.
For the time beeing you should not use class.proxy setting under ODMG.
]

 As I looked into the
> internals of ojb I came across the following explanation for the
> abovementioned behaviour:
> 
> 
> 	a) Thread-1 obtains a Write lock
> 	b) FieldB is changed to B, update statement prepared (but not yet
> 	commited in database (==> in database FieldB [actually the
> 		Fk-Field, but for the rest of this I will just say
> 		FieldB] is still null)


==> On changing fieldB the proxy *must* materialize the real object if 
not done before.


> 	c) Thread-2 tries to obtain a read lock which rightly
> 	(repeatable-read) is not granted => in TransactionImpl.java the
> 	following happens:
> 		throw new LockNotGrantedException("Can not lock " + obj +
> 		" for READ");
> 
> 	+ obj + means that java will call obj.toString(), which means that
> 	since obj is a proxy it gets delegated to the IndirectionHandler,
> 	which (if realSubject hasn't been materialized yet) will
> 	materialize the realSubject. This is done by loading it from the
> 	database (Thread-1 removed it from the cache before). BUT, in
> 	the database there is still the old value of FieldB, because
> 	Thread1 hasn't commited the database-transaction yet. The loaded
> 	object is then put into the cache.
> 
> 	d) The database-transaction in Thread-1 commits the new value for
> 	fieldB, but since it was just put into the cache with the old
> 	values subsequent queries to it, still have the old value.


Mh, on commiting objects a PB.invalidate(obj) is performed that should 
avoid this behaviour ???


> 
> 	e) I just put throw new LockNotGrantedException() to stop obj from
> 	materializing. This reduced the incident of the aforementioned
> 	problem, but it didn't stop it completly, so I assume there are
> 	other parts of the code which show a similar effect.
> 


The best thing to get clearity about your analysis is to provide a JUnit 
testcase that reproduces this behaviour!
Without such a testcase it's really difficult to verify your report.


> 
> 3.) On debugging the just mentioned problem I stumbled across the
> following:
> 
> 	In LockStrategyFactory.java getStrategyFor(Object obj) a
> 	LockStrategy is assigned by looking up obj.getClass(), which for
> 	a dynamic Proxy is not the actual class ==> the default
> 	ReadUncommitedStrategy is assigned for every proxy. The following
> 	lines will change this:
> 
> 	Instead of using:
> 		        int isolationLevel = getIsolationLevel(obj.getClass());
> 
> 	do:
> 	if (obj instanceof Proxy) {
> 	    InvocationHandler ih = Proxy.getInvocationHandler(obj);
> 
> 	    if ((ih != null) && (ih instanceof IndirectionHandler)) {
> 		c = ((IndirectionHandler) ih).getIdentity().getObjectsClass();
> 	    } else {
> 		//System.err.println("UPGRADING PROXY FAILED: " + c);
> 		//TODO: handle this error case
> 	    }
> 
> 	}
> 
> 	if (c == null) {
> 	    c = obj.getClass();
> 	}
> 
> 	int isolationLevel = getIsolationLevel(c);
> 


This seems to be a bug. Thanks for your patch !


> 
> 4.) The following observation is just anecdotal, It seems that when
> querying for an object which is a proxy with e.g.:
> 
> 	DList l = (DList) query.execute();
> 
> 	if (l.size() == 1) {
> 
> 	tx.abort;
> 	return;
> 	}
> 
> 
> 	It seems that read-locks associated with the 1 result are not
> 	being released (the ObjectEnvelopeTable doesn't have any entries)
> 
> 	My only solution is to actually materialize the queried subject
> 	by doing the following:
> 
> 	SomeClass A = (SomeClass) l.get(0);
> 
> 	and then calling a method on it
> 
> 	=> object is materialized and is in the ObjectEnvelopeTable and
> 	read-locks are release when calling tx.abort();
> 


This is a again the "lazy-locking" effect mentioned above. Works as 
designed !


> 
> 5.) A general question about locking. Is there any possibility you are
> going to implement some kind of blocked locking (i.e. a thread simply
> waits for a lock to become available), instead of throwing a
> LockNotGrantedException?
> 


It won't be difficult to implement such a mechanism.
But ODMG wants the implemtor to throw exceptions if Locking fails.
We simply stick to the rules here.


HTH, thanks for your observance to details, 


Thomas