2012/3/15 Patrick Ohly <[email protected]>:
> On Thu, 2012-03-15 at 10:34 +0100, Krzesimir Nowak wrote:
>> 2012/3/14 Patrick Ohly <[email protected]>:
>> > Hello Chris!
>> >
>> > You added a callback parameter to addResource() (formerly known as
>> > enqueue()) and related functions. There's no documentation for that in
>> > server.h/cpp. What is it meant to be used for, what kind of constraints
>> > does it have, etc.? In other words, please document it for me... ;-)
>> >
>>
>> That is my doing again. Will document it. The callback was added,
>> because addResource calls checkQueue() which calls SetActive, which is
>> asynchronous. The callback is called when after all SetActive calls
>> are finished or, if there was no call to SetActive, at the end of
>> checkQueue(). In short - addResource, removeResource are asynchronous
>> too, so given callback is called when they are finished.
>
> Instead of just calling the parameter "callback" it would be good to use
> a name which is related to the time when the callback is invoked.
Well, maybe - the callback is called after in fact adding or removing
resource is finished. I have no idea what other name it should have. I
will have to just document it when it may be called.
> The goal of the callback is to return the final result of a D-Bus method
> invocation to the D-Bus client, right?
Not really - is more general. It may be your main use, but not
necessarily. I mean - SetActive returns nothing, but some code might
need waiting for those calls to be finished.
> Why does that have to wait for the completion of SetActive? Can't we
> tell the D-Bus client that its StartSession() was successful, then later
> emit the right "status changed" signal?
If you don't care about when SetActive finishes and want to proceed
then passing nullCb should be enough.
>
> The code is here:
> // If not we activate the resource and place it in the active
> resource list.
> if (canRun) {
> m_activeResources.push_back(waitingResource);
> // If this is a session, we set active and emit sessionChanged
> to clients.
> boost::shared_ptr<SessionResource> session =
>
> boost::dynamic_pointer_cast<SessionResource>(wq_iter->lock());
> if (session) {
> ++(*counter);
> session->setActiveAsync(true,
> boost::bind(&Server::setActiveCb, this, session, counter, callback));
> }
> m_waitingResources.erase(wq_iter++);
> }
>
> That seems unnecessarily complex to me, but perhaps I am missing something.
That was the case that if two sessions are going to be active then we
don't want to have our callback called twice.
Of course, if you feel that all of this is not necessary and
overcomplicated, then I will just remove it.
Previously calling SetActive had immediate effect, so we could call
e.g. SetNamedConfig right after it. Now it involves DBus, so maybe
some code will have to wait until the session is really active. I
probably have written this code when I was tracking that race
condition in GIO GDBus.
>
> --
> Best Regards, Patrick Ohly
>
> The content of this message is my personal opinion only and although
> I am an employee of Intel, the statements I make here in no way
> represent Intel's position on the issue, nor am I authorized to speak
> on behalf of Intel on this matter.
>
>
_______________________________________________
SyncEvolution mailing list
[email protected]
http://lists.syncevolution.org/listinfo/syncevolution
