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


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!


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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]