[Mono-list] Class library developers: locking issues to keep in mind

[Serge]

> Clever, but broken :)
Well, let's try to repair ;-)

Let's use Java model, since NET's locking semantics are not clear at the
moment :)
Quotes are taken from Doug Lea's "Concurrent Programmin in Java (2nd ed.)"
chapter 2.2.7.2 pp 94-95, this chapter is also available online ([1]).

> Flaw #1: Now, the order in which JVM sends updates from local memory to
> shared mememory is arbitrary.
That's right. Good point.
"If a field is declared as volatile, any value written to it is flushed and
made visible by the writer thread before the writer thread performs any
further memory operation (i.e., for the purposes at hand it is flushed
immediately)."
So declaring them volatile would help (but see below, flaw #2). And Miguel
confirmed that volatile has essentially the same semantics in .NET.

> Flaw #2: Assuming that initializer points to DummyInit, singleton may
> still point not to null but to a incompletely initialized instance of
> Singleton instance
Nope, there are only two possibilities - either fully initialized singleton
or null. Instance is only created inside the fully synchronized method
(RealInit):
"In essence, releasing a lock forces a flush of all writes from working
memory employed by the thread, and acquiring a lock forces a (re)load of the
values of accessible fields." But this is only in the case if "writing
thread releases a synchronization lock and a reading thread subsequently
acquires that same synchronization lock", so you're right, due to
unspecified flushing sequence another thread that sees DummyInit, could see
(singleton == null) but never incompletely initialized singleton.
Volatile should help with that (changes are immediately visible).
Other possible fixes (in absence of volatile for example, or if volatile
doesn't guaranties immediate flushing):
1) Add yet another check:
     public static object GetHeavyObject ()
     {
          // calling either RealInit or DummyInit
          initializer  ();

          // at this point singleton is either null
          // or fully initialized object

          // now check it again
          if (singleton == null) RealInit ();

          // if RealInit invoked here, then
          // all values will be reloaded.
          // That's because RealInit is fully-synched,
          // see quote for Flow 2

          return singleton;
     }

It is safe to call RealInit, because it is a) fully-synchronized b) contains
it's own check so singleton will be created only once in any case.

2) Spawn yet another thread inside the RealInit, that will perform
initialization and exit. Then Join.
"As a thread terminates, all written variables are flushed to main memory."
Of course, thread only spawned if (singleton == null)

> Flaw #3: A minor one. The order of sending updated to shared memory
> might be vice versa: first singleton, then initializer. In that case,
> RealInit might be executed several times for one thread, because
> initializer is not always changed to DummyInit inside RealInit. If
> singleton is not null, initializer will still keep on poiting to ReadInit.

Nope, it will never execute more than once for one thread - RealInit is
entirely locked, if (sinleton == null) branch is only executed exactly one
in any case.
However, different thread(s) could enter RealInit before it's completed, it
will wait on method's lock held by the first thread. After the first thread
finishes executing RealInit and releases lock all changes are visible to the
second thread (see quote from Flow 2).
Actually, this one is the main problem. Theoretically, there could be
arbitrary number of thread waiting for RealInit lock. Those are the threads
started during the time RealInit's body is executed. This will work
correctly though - RealInit acts just like simple initializer with
serialized access (without DCL). But this will have performance impact of
course.

Anyhow, I think either volatile access or above two fixes should eliminate
any issues (sure, it's quite possible that I'm wrong :-)

Any thoughts?

Sergey


[1] http://gee.cs.oswego.edu/dl/cpj/jmm.html