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

[Ferdinando A. <na...@am...>]

Hi all

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 ?

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