Re: [Quantlib-dev] QuantLib-SWIG, the observer pattern and destructor call during update

[Luigi B. <lui...@gm...>]

On Wed, Feb 29, 2012 at 4:51 PM, Ferdinando Ametrano <na...@am...> wrote:
> I wonder if it would be more effective to just live with the fact that
> QuantLib 1.x is not thread-safe and start a parallel
> non-backward-compatible QuantLib 2.x ?

That's a possibility (and seeing the number of issues we'll have to
address for 2.0, we'll also have to discuss whether to keep using
Subversion or move to a DVCS).

However, the problem is that even if one doesn't use threads, as soon
as one uses QuantLib in Java or C# the system is going to run the
garbage collector in another thread.
So until a 2.0 is out, version 1.x is pretty much unusable from those
languages for anything else than toy programs.

Luigi


> We might use 2.x for an overall library re-factoring, with the spotlight on
> 1) thread-safety
> 2) parallelism
> 3) increased boost-ification (math, distributions, date, random
> numbers, optional, etc)
> 4) review of few issues about Instrument/Pricing engines, etc
> 5) interface clean-up removing methods which can be implemented as functions
> 6) review of the worst performance bottlenecks (e.g. virtual
> Event::hasOccurred, etc)
> 7) fix the non invertible relation between dates and times in TermStructure
> 8) etc
>
> ciao -- Nando
>
> On Wed, Feb 29, 2012 at 12:45 PM, Luigi Ballabio
> <lui...@gm...> wrote:
>> Hi all,
>>    recently Klaus Spanderen has posted on his blog about the
>> Observer/Observable problems we had when QuantLib is exported through
>> SWIG to a language like Java or C# which run garbage collection in a
>> separate thread (see
>> <http://old.nabble.com/Issues-with-C--Swig-Bindings,-NUnit-and-Settings.instance%28%29.setEvaluationDate%28%29-td30549787.html>
>> for the original thread and
>> <http://hpcquantlib.wordpress.com/2012/02/27/quantlib-swig-and-a-thread-safe-observer-pattern-in-c/>
>> for Klaus' post).
>>
>> I've been thinking about it for the last few months, too.  I haven't a
>> full solution (I guess the real solution would be to rewrite the whole
>> thing in terms of Boost.Signal2, which is thread-safe) but here's what
>> I got so far.
>>
>> The original problem is that we've gone against one of the basic
>> tenets of C++, namely, that release of resources should go in the
>> inverse order as their acquisition. What happens now is that observers
>> unregister themselves in the destructor they inherit from the base
>> Observer class, which results in the following sequence of actions:
>>
>> On construction:
>> - call the Observer constructor, which builds the base-class part of
>> the instance;
>> - call the derived-class constructor, which builds the derived-class
>> attributes and stuff;
>> - inside the derived-class constructor, register with the observables.
>>
>> On destruction:
>> - call the derived-class destructor, which destroys the derived-class
>> attributes and stuff;
>> - call the Observer destructor, which unregisters with the observables...
>> - ...and then destroys the base-class part of the instance.
>>
>> The problem is that we have a-b-c during construction and b-c-a during
>> destruction (it should be c-b-a).  Doing it this way, sometimes it
>> happens that an observable sends a notification between b and c. The
>> observer is not yet unregistered, so it gets the notification; but
>> since it's already been partially destroyed, the resulting call to
>> update() results in a crash.
>>
>> Both Henner and Klaus did their best to fix the Observer class, but I
>> think the solution is to do things in the correct order (c-b-a), that
>> is, unregister in the destructor of the derived class.  But how?
>> Forcing one to write the calls to unregisterWith() inside the
>> destructor (and often, to write an explicit destructor just for that)
>> cannot be enforced, and would result in dangling pointers as soon as
>> one forgets to do it.
>>
>> One way might be to use RAII to do this.  We might implement a helper
>> class like:
>>
>> template <class T>
>> class Registered {
>>    T observable_;
>>    Observer* observer_;
>>  public:
>>    Registered(const T& observable, Observer* observer)
>>    : observable_(observable), observer_(observer) {
>>        observer_->registerWith(observable_);
>>    }
>>    ~Registered() {
>>        observer_->unregisterWith(observable_);
>>    }
>>    const T& operator->() const { return observable_; }
>> };
>>
>> that wraps an observable and manages unregistration.  This way,
>> instead of writing derived observer classes as:
>>
>> class SomeClass {
>>    Handle<YieldTermStructure> ts_;
>>  public:
>>    SomeClass(const Handle<YieldTermStructure>& ts) : ts_(ts) {
>>        registerWith(ts_);
>>    }
>> };
>>
>> we would write:
>>
>> class SomeClass {
>>    Registered<Handle<YieldTermStructure> > ts_;
>>  public:
>>    SomeClass(const Handle<YieldTermStructure>& ts) : ts_(ts, this) {}
>> };
>>
>> This way, ts_ is a Registered instance (which provides an
>> operator->(), so it can be used as before; for instance,
>> ts_->discount(t) still works) and when SomeClass is destroyed, the
>> unregisterWith call in the Registered destructor will fire during the
>> destruction of SomeClass, doing things in the correct order.
>>
>> (Note: it would also be possible to work out things so that we can
>> leave the registerWith() call in the constructor, if we want to modify
>> the least possible amount of code. When called with a Registered as an
>> argument, it would register the observer with the wrapped observable
>> and store the observer's "this" pointer into the Registered instance.)
>> (Note 2: "Registered" might not be the best name.  "Observed", maybe?)
>>
>>
>> Unfortunately, it's not foolproof.  For instance, the Registered
>> instances are better declared last in the class; and even in that
>> case, if we have two Registered (A and B) there might be freak
>> scenarios in which A is unregistered and destroyed, and before B can
>> be unregistered it fires a notification.  If the observer's update()
>> method just flips a bool, it will work fine; but if update() tries to
>> access A instead, it will find it destroyed and hilarity will ensue.
>>
>> Also, we would still have the problem that an observer might be
>> removed from an observable's registered list while the observable is
>> iterating over it in notifyObservers().  We'll need a lock to prevent
>> that.
>>
>> I guess this about wraps it up.  Thoughts?
>>
>> Later,
>>    Luigi
>>
>> ------------------------------------------------------------------------------
>> Virtualization & Cloud Management Using Capacity Planning
>> Cloud computing makes use of virtualization - but cloud computing
>> also focuses on allowing computing to be delivered as a service.
>> http://www.accelacomm.com/jaw/sfnl/114/51521223/
>> _______________________________________________
>> QuantLib-dev mailing list
>> Qua...@li...
>> https://lists.sourceforge.net/lists/listinfo/quantlib-dev