Hello Chris!
Your patch removes the Server::m_shutdownSession and replaces it with a
boolean m_shutdownRequested + a timeout that is set once in
Server::fileModified().
The comment in server.h still refers to m_shutdownSession and explains
that its purpose was to prevent other sessions from running. How is that
handled now?
More specifically, consider the following sequence of events:
1. files are modified and the quiesence timeout is started
2. a client connects, starts a session and syncs
3. the timeout triggers, causing the server to shut down
If it works as designed (we don't have tests for it, do we?), then the
syncevo-dbus-helper will continue to run and complete the session. But
the client will see an unexpected server disconnect and won't be able to
monitor progress of the still running session, because the running
helper isn't connected to any syncevo-dbus-server instance.
It could even be worse: with bad timing, an obsolete syncevo-dbus-helper
might get started which then fails because it doesn't match the
installed files.
IMHO adding any kind of resource should be prevented while the
m_shutdownRequested flag is set. Returning an error message to the
client would be okay and in fact better than the previous behavior
(letting it create a session, then disconnect without any specific error
message).
Do you agree with this analysis or did I miss something?
Don't bother changing code, I'm just trying to clarify the existing one.
--
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
