D-Bus server: fix unreliable shutdown handling

Occassionally, syncevo-dbus-server locked up after receiving
a CTRL-C. This primarily affected nightly testing, in particular (?)
on Ubuntu Lucid.

The reason was two-fold:
- indirectly allocating memory when getSuspendFlags() had to
  allocated the guard instance of the mutex
- glib mutex held by some other thread when trying to do
  g_main_loop_quit().

SuspendFlags already handles signals without these problems. So
instead of overriding that, let SuspendFlags signal the receival
of a signal by writing to its FD and monitor that in our main
event loop, then quit the main event loop from inside the main
thread itself.
This commit is contained in:
Patrick Ohly 2014-10-24 05:25:04 -07:00
parent efedc4d4f1
commit c2dd6033ed
3 changed files with 37 additions and 18 deletions

View File

@ -45,16 +45,8 @@ using namespace GDBusCXX;
namespace {
GMainLoop *loop = NULL;
bool shutdownRequested = false;
const char * const execName = "syncevo-dbus-server";
void niam(int sig)
{
shutdownRequested = true;
SuspendFlags::getSuspendFlags().handleSignal(sig);
g_main_loop_quit (loop);
}
bool parseDuration(int &duration, const char* value)
{
if(value == NULL) {
@ -189,10 +181,6 @@ int main(int argc, char **argv, char **envp)
// process name for developers in this process, and not in
// syncevo-dbus-helper.
Logger::setProcessName("syncevo-dbus-server");
SE_LOG_DEBUG(NULL, "syncevo-dbus-server: catch SIGINT/SIGTERM in our own shutdown function");
signal(SIGTERM, niam);
signal(SIGINT, niam);
boost::shared_ptr<SuspendFlags::Guard> guard = SuspendFlags::getSuspendFlags().activate();
DBusErrorCXX err;
@ -206,7 +194,7 @@ int main(int argc, char **argv, char **envp)
// make this object the main owner of the connection
boost::scoped_ptr<DBusObject> obj(new DBusObject(conn, "foo", "bar", true));
boost::shared_ptr<SyncEvo::Server> server(new SyncEvo::Server(loop, shutdownRequested, restart, conn, duration));
boost::shared_ptr<SyncEvo::Server> server(new SyncEvo::Server(loop, restart, conn, duration));
server->setDBusLogLevel(levelDBus);
server->activate();
@ -225,13 +213,13 @@ int main(int argc, char **argv, char **envp)
#endif
server.reset();
obj.reset();
guard.reset();
SE_LOG_DEBUG(NULL, "flushing D-Bus connection");
conn.flush();
conn.reset();
SE_LOG_INFO(NULL, "terminating, closing logging");
syslogger.reset();
redirect.reset();
guard.reset();
SE_LOG_INFO(NULL, "terminating");
return 0;
} catch ( const std::exception &ex ) {

View File

@ -345,7 +345,6 @@ void Server::getSessions(std::vector<DBusObject_t> &sessions)
}
Server::Server(GMainLoop *loop,
bool &shutdownRequested,
boost::shared_ptr<Restart> &restart,
const DBusConnectionPtr &conn,
int duration) :
@ -354,7 +353,8 @@ Server::Server(GMainLoop *loop,
SessionCommon::SERVER_IFACE,
boost::bind(&Server::autoTermCallback, this)),
m_loop(loop),
m_shutdownRequested(shutdownRequested),
m_suspendFlagsSource(0),
m_shutdownRequested(false),
m_restart(restart),
m_conn(conn),
m_lastSession(time(NULL)),
@ -411,8 +411,31 @@ Server::Server(GMainLoop *loop,
m_configChangedSignal.connect(boost::bind(boost::ref(configChanged)));
}
gboolean Server::onSuspendFlagsChange(GIOChannel *source,
GIOCondition condition,
gpointer data) throw ()
{
Server *me = static_cast<Server *>(data);
try {
if (!SuspendFlags::getSuspendFlags().isNormal()) {
me->m_shutdownRequested = true;
g_main_loop_quit(me->m_loop);
SE_LOG_INFO(NULL, "server shutting down because of SIGINT or SIGTERM");
}
} catch (...) {
Exception::handle();
}
// Keep watching, just in case that we catch multiple signals.
return TRUE;
}
void Server::activate()
{
// Watch SuspendFlags fd to react to signals quickly.
int fd = SuspendFlags::getSuspendFlags().getEventFD();
GIOChannelCXX channel(g_io_channel_unix_new(fd), TRANSFER_REF);
m_suspendFlagsSource = g_io_add_watch(channel, G_IO_IN, onSuspendFlagsChange, this);
// Activate our D-Bus object *before* interacting with D-Bus
// any further. Otherwise GIO D-Bus will start processing
// messages for us while we start up and reject them because
@ -446,6 +469,9 @@ void Server::activate()
Server::~Server()
{
// make sure all other objects are gone before destructing ourselves
if (m_suspendFlagsSource) {
g_source_remove(m_suspendFlagsSource);
}
m_syncSession.reset();
m_workQueue.clear();
m_clients.clear();

View File

@ -64,7 +64,8 @@ class ServerLogger;
class Server : public GDBusCXX::DBusObjectHelper
{
GMainLoop *m_loop;
bool &m_shutdownRequested;
guint m_suspendFlagsSource;
bool m_shutdownRequested;
Timespec m_lastFileMod;
boost::shared_ptr<SyncEvo::Restart> &m_restart;
GDBusCXX::DBusConnectionPtr m_conn;
@ -417,9 +418,13 @@ class Server : public GDBusCXX::DBusObjectHelper
/** hooked into m_idleSignal, controls auto-termination */
void onIdleChange(bool idle);
/** hooked up to SuspendFlags fd via g_io_add_watch */
static gboolean onSuspendFlagsChange(GIOChannel *source,
GIOCondition condition,
gpointer data) throw ();
public:
Server(GMainLoop *loop,
bool &shutdownRequested,
boost::shared_ptr<Restart> &restart,
const GDBusCXX::DBusConnectionPtr &conn,
int duration);