[Geotools-devel] Why we have two locks

[Jody G. <jga...@re...>]

Martin Desruisseaux wrote:
> Sigh... One of us is really not understanding the other one point. I 
> claim again that *the read lock in ObjectCacheEntry is totally 
> useless* in order to prevent the same CRS to be built twice in two 
> different threads. Did you understand the workflow I posted two days 
> ago? (posting again below). Or does my workflow has a flaw?
Agreed - sigh... I have about five of your messages on a "todo" tag ... 
since this is causing us grief I will go through this workflow now. And 
I will also use it to construct a test case for later.
I will document the results of our discussion here:
- http://docs.codehaus.org/display/GEOTDOC/4+Handling+Many+Threads

I am going to number the lines using good old fashioned BASIC 
conventions. (Ah Apple//e where are you).
> 10 - thread 1 calls get(key) returns null from cache miss
> 20 - thread 1 acquires the lock
25 - thread 1 checks peek(key) to ensure the value is still null
> 30 - thread 1 starts building the referencing object
> 40 - thread 2 get(key) returns null from cache miss
> 50 - Thread 2 try to acquire the lock
> 60 - Thread 2 block because thread 1 already has the lock
> 70 - thread 1 finishes building the referencing object
> 80 - thread 1 stores the value in the cache
> 90 - thread 1 releases the lock
> 100 - thread 2 acquires the lock
> 110- thread 2 realize that the object is already created, because it 
> performs a call to peek(key) once it hold the lock and this second 
> call returns a non-null object
> 120 - Thread 2 releases the lock. *Thread 2 has NOT recreated the CRS 
> - it got if from the cache*
Reading your workflow now and it looks fine - it shows that the cache 
can be kept consistent in the face of multiple threads.  The problem we 
are trying to solve is not only cache consistency, but allowing multiple 
workers to be in flight at the same time. This is required so we can 
have each working on creating different referencing objects, 
*concurrently*. The reason we have two locks in use is in order to 
prevent several workers being dispatched to create the same object.

If we just had a single lock on the map we would only be able to work on 
a creating a single referencing object at a time. By having a second 
lock on the "map entry" we are able to have several workers going at 
once - each working on a defining a different value.

The following uses the same line numbers as above (in case you want to 
compare); as we would not see benifit of two locks without more threads 
I have added thread 3 into the mix

1 - thread 1 create( key ) is called
2 - thread 1 create() calls get( key )
3 - thread 1 get() synchronizes on the internal map
4 - thread 1 get() does a containsKey( key ) check and discovers that 
there is no ObjectCacheEntry for the key
5 - thread 1 get() leaves synchronization on the internal map
> 10 - thread 1 get(key) returns null from cache miss
11 - thread 1 create() retrieves one worker from the object pool in 
order to create this value
12 - thread 1 create() calls the worker create() method
13 - thread 1 worker create() calls get( key )
14 - thread 1 worker get() synchronizes on the internal map
15 - thread 1 worker get() does a containsKey( key ) check and discovers 
that there is no ObjectCacheEntry for the key
16 - thread 1 worker get() leaves synchronization on the internal map
17 - thread 1 worker get() returns null from cache miss

20 - thread 1 worker create() calls writeLock( key )
21 - thread 1 worker writeLock( key ) synchronizes on the internal map
22 - thread 1 worker writeLock( key ) calls containsKey and discovers 
that there is no ObjectCacheEntry for the key
23 - thread 1 worker writeLock( key ) creates a ObjectCacheEntry for the key
24 - thread 1 worker writeLock( key ) leaves synchronization on the 
internal map
24 - thread 1 worker writeLock( key ) grabs the write lock on the 
ObjectCacheEntry for the key
30 - thread 1 worker create() starts building the referencing object ...

39 - thread2 create( key ) is called
> 40 - thread 2 create() calls get( key )
41 - thread 2 get() synchronizes on the internal map
42 - thread 2 get() does a containsKey( key ) check and discovers that 
is a ObjectCacheEntry for the key
43 - thread 2 get() leaves synchronization on the internal map
44 - thread 2 get() calls on ObjectCacheEntry.getValue()
45 - thread 2 get() blocks on the readLock

46 - thread 3 calls get( key2 )
47 - thread 3 get() synchronizes on the internal map
48 - thread 3 get()  does a containsKey( key2)  check and discovers that 
there is no ObjectCacheEntry for the key2
49 - thread 3 get() leaves synchronization on the internal map
50 - thread 3 get() returns null from cache miss
51 - thread 3 retrieves one worker from the object pool in order to 
create this value
52 - thread 3 calls the worker create() method
53 - thread 3 worker7 create() calls get( key2 )
54- thread 3 worker7 get() synchronizes on the internal map
55 - thread 3 worker7 get() does a containsKey( key2 ) check and 
discovers that there is no ObjectCacheEntry for the key2
56 - thread 3 worker7 get() leaves synchronization on the internal map
57 - thread 3 worker7 get() returns null from cache miss
58 - thread 3 worker7 calls writeLock( key2 )
59 - thread 3 worker7 writeLock( key2 ) synchronizes on the internal map
60 - thread 3 worker7 writeLock( key2 ) calls containsKey( key2 ) and 
discovers that there is no ObjectCacheEntry for the key2
61 - thread 3 worker7 writeLock( key2 ) creates a ObjectCacheEntry for 
the key2
62 - thread 3 worker7 writeLock( key2 ) leaves synchronization on the 
internal map
63 - thread 3 worker7 writeLock( key2 ) grabs the write lock on the 
ObjectCacheEntry for the key2
64 - thread 3 worker7 create() starts builds the referencing object
65 - thread 3 worker7 create() stores the value in the ObjectCacheEntry
66 - thread 3 worker7 create() releases the writeLock
67 - thread 3 worker7 create() returns the created value
68 - thread 3 create() returns the created value
> 70 - thread 1 the worker create() finishes building the referencing 
> object
> 80 - thread 1 the worker create() stores the value in the ObjectCacheEntry
> 90 - thread 1 the worker create() releases the writeLock (all readers 
> including Thread2 can now proceed)
> 91 - thread 1 the worker create() returns the created value
> 92 - thread 1 returns the created value
> 100 - thread 2 get() is aquires the readLock
> 101 - thread 2 get() retrives the ObjectCacheEntry.getValue()
> 102 - thread 2 get() releases the readLock
> 110 - thread 2 create() realize that the object is already created
> 120 thread 2 create() returns the object it found in the cache 
> //Thread 2 has NOT recreated the CRS - it got if from the cache 
It is the relationship between the mediator and the workers that require 
that two locks be used. We are careful to always:
a) access to the internal map in synchronized blocks
b) access to ObjectCacheEntry.getValue / setValue is protected by a read 
write lock

We are careful to leave the synchronization of the internal map block 
before entering any read/write lock on the ObjectCacheEntry.

Cheers,
Jody