2012/3/15 Patrick Ohly <patrick.ohly(a)intel.com>:
ConnectionResource and SessionResource share a lot of code: init(),
onQuit(), onFailure(), ... - basically the whole startup of the helper
seems to have been copied. Shouldn't that common logic be in the
Resource base class?
Maybe some of the code could be common, but not that whole. Proxy
handling is different for Session and Connection. Different
environment variables are used. onQuit and onFailure can be easily
moved. I will do it, but maybe bit later when more pressing issues are
I also wonder about object ownership while the resource is still in the
process of getting constructed.
void SessionResource::createSessionResource(const Callback_t &callback,
const std::string &peerDeviceID,
const std::string &configName,
const std::string &session,
std::auto_ptr<SessionResource> resource(new SessionResource(server,
// init did not throw an exception, so we guess that child was
// spawned successfully. thus we release the auto_ptr, so it will
// not delete the resource.
"we guess" is not encouraging when it comes to memory ownership ;-} Do
we know for sure? If createSessionResource() transfers owership to
init(), then it should do so explicitly.
Ah, well, exception was not thrown at this point so I stated that all
went fine. The situation when resource could leak would be when there
was some failure during DBus communication between sync-helper and
server e.g. sync-helper crashed. I know - it is still ugly, so I will
change that it will return a shared_ptr to Session, but it will have
to be documented that such Session is not very usable at this point.
To use such Session (or Connection) server has to wait until callback
Next, SessionResource::init(). It connects the resource to ForkExec
signals, using a plain pointer to the resource, without the
boost::signal2 track mechanism that I had suggested.
With creation of shared_ptr above it should be easy to solve now.
Who owns the SessionResource at that point? Who is going to free it
something goes wrong before (eventually) addResource() is called? As far
as I can tell, the instance will simply leak.
Another situation is when the D-Bus client quits while its requested
session still gets constructed. The Client instance of the client will
get destructed. But the SessionResource was not yet connected to it, so
the construction cannot be aborted by Server.
I still think resources should be added to m_waitingResources before
construction is complete. Activating the object and notifying the caller
can wait until the resource is really usable (= helper ready).
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.