[Ekiga-devel-list] Code in lib/engine/components/opal/

[Julien Puydt <jpuydt free fr>]

Hi,

I just had a deeper look at what the code in this directory does (and 
commit something which shouldn't change things much, we just use 
Ekiga::ServiceCore only at startup).

In Opal::Sip::EndPoint::update_bank, I see that the endpoint is 
connecting to Opal::Bank's signals to know what happens ; which means 
it's "above" it from an abstract point of view. This is confirmed by 
inspection of methods Opal::Sip::EndPoint::menu_builder_add_actions and 
Opal::Sip::EndPoint::account_updated_or_removed, which ping the bank to 
know what to do. The endpoints also asks things to Opal::SIP::Dialect!

On the other hand, Opal::Bank delegates everything to Opal::Account. And 
what does Opal::Account do? Whenver it has something to do, it asks 
Opal::Sip::EndPoint!


In short, the code in this directory is structurally bound to give problems.


I suggest the following organization:

- Opal::Sip::EndPoint does everything low-level (register/unregister 
accounts, call and presence management) ; when something happens, it 
emits signals. It shouldn't ping the bank for anything, as it's not its 
business!

- Opal::Account is connected on the endpoint, and gives orders 
(registration, presence subscription/unsubscription), and watches the 
signals to know when something happens (registration, mwi, presence) (it 
is "above" abstractly).

- Opal::SIP::Dialect should also be "above" the endpoint : when it wants 
to send a message, it asks the endpoint, and when a message comes, it 
knows about it through a signal.

- Opal::Bank should become 
Ekiga::PresentityDecorator+Ekiga::ContactDecorator, and when one of the 
populate_menu methods gets called, loop on the account, and call their 
own populate_menu methods, which will then call a method in the 
endpoint, which will actually add actions (to each its own business!)


The big picture is that I would like to replace the Gordian knot that we 
now have with a nice tree (ie: no dependency cycle) :

Opal::CallManager -----------\
                              \
Opal:: Bank -> Opal::Account -->- Opal::Sip::EndPoint
                               /
Opal::SIP::Dialect -----------/


The current memory handling is also unsatisfying (cf opal-main.cpp), 
since we put the endpoints in boost shared pointers, but with a 
null_deleter, which means we don't really manage them. This hits the 
current ekiga master : as Damien noted (in a private email), the 
endpoints get freed before the accounts, so the accounts call some 
endpoint and we get a crash! Somewhere else in ekiga, there must be some 
code which says to dispose of the endpoints ; that is wrong.

The good solution is to remove that other code, and put our endpoints in 
the boost shared pointers with a deleter which will shut them down 
gently, then call delete. This way, we'll be sure that the endpoints 
will live as long as something in the ekiga needs them, and that they 
are properly closed whenever they're not needed anymore. It's the code 
which creates the objets, which should take care of disposing of the 
objects!

Snark

(PS: notice how we sometimes have SIP and sometimes Sip in the namespaces)