6:16 a.m.
On Fri, 2012-03-16 at 13:11 +0100, Krzesimir Nowak wrote:
2012/3/15 Patrick Ohly <patrick.ohly(a)intel.com>:
> On Thu, 2012-03-15 at 10:34 +0100, Krzesimir Nowak wrote:
>> 2012/3/14 Patrick Ohly <patrick.ohly(a)intel.com>:
>> > 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.
In SessionResource for createSessionResource(), init(), and
onSessionConnect() a better name would be "constructionCompleted". Note
my previous comments about the lack of a "constructionFailed".
In Server, in checkQueue() and addResource() use the same kind of
callback, but I don't understand what kind of guarantee is made about
it. Something like "activatedOrFailed"?
It's not the same as SessionResource::setActiveAsyncCb's "callback",
although it has the same name and signature.
> 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.
If I knew what the callback stands for, then perhaps I would understand
the code :-/
--
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.