shill: Prevent silent discarding of GLib IO channel errors
Shill uses GLib's IO channels to poll some of its file descriptors, like
the RTNL socket. On detecting an error condition on the socket the
callbacks just return FALSE (causing GLib to unregister the descriptor)
without any further action or output. The process keeps on running
without receiving new netlink messages, and the upper layers are never
even informed of the condition.
For the descriptors currently managed through GLib, error conditions on
poll() or read() usually indicate a system-wide failure or bug that
should not happen under normal conditions. One such current issue is in
the kernel's NETLINK_ROUTE system, which can occasionally produce errors
in a rare and hard to reproduce manner. This condition does not seem to
immediately happen again when opening a new RTNL socket.
This patch changes the affected GLib callback handlers to always log an
error message and die on these kinds of error conditions. This seems to
be the most practical answer to these unexpected and rare occurences,
forcing a full restart of shill through upstart which is probably our
best bet to recover from them. The callback handlers no longer return
FALSE under any circumstances.
BUG=chromium-os:36328
TEST=Hack the code to purposefully trigger one of the LOG(FATAL) code
paths after some time. Watch shill die (with a sightly misformatted
backtrace) and get immediately restarted.
Change-Id: I74e47ab41a029d0b4fc509c525cb5cb86a871a2b
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/37980
Reviewed-by: mukesh agrawal <quiche@chromium.org>
diff --git a/glib_io_input_handler.cc b/glib_io_input_handler.cc
index e63fc67..7eafa05 100644
--- a/glib_io_input_handler.cc
+++ b/glib_io_input_handler.cc
@@ -4,7 +4,9 @@
#include "shill/glib_io_input_handler.h"
+#include <errno.h>
#include <stdio.h>
+#include <string.h>
#include <glib.h>
#include "shill/logging.h"
@@ -19,24 +21,30 @@
GlibIOInputHandler *handler = reinterpret_cast<GlibIOInputHandler *>(data);
unsigned char buf[4096];
gsize len = 0;
- gboolean ret = TRUE;
+ gint fd = g_io_channel_unix_get_fd(chan);
+ GError *err = 0;
if (cond & (G_IO_NVAL | G_IO_HUP | G_IO_ERR))
- return FALSE;
+ LOG(FATAL) << "Unexpected GLib error condition " << cond << " on poll("
+ << fd << "): " << strerror(errno);
GIOStatus status = g_io_channel_read_chars(
- chan, reinterpret_cast<gchar *>(buf), sizeof(buf), &len, NULL);
- if (status != G_IO_STATUS_NORMAL) {
- if (status == G_IO_STATUS_AGAIN)
- return TRUE;
- len = 0;
- ret = FALSE;
+ chan, reinterpret_cast<gchar *>(buf), sizeof(buf), &len, &err);
+ if (err) {
+ LOG(WARNING) << "GLib error code " << err->domain << "/" << err->code
+ << " (" << err->message << ") on read(" << fd << "):"
+ << strerror(errno);
+ g_error_free(err);
}
+ if (status == G_IO_STATUS_AGAIN)
+ return TRUE;
+ if (status != G_IO_STATUS_NORMAL)
+ LOG(FATAL) << "Unexpected GLib return status " << status;
InputData input_data(buf, len);
handler->callback().Run(&input_data);
- return ret;
+ return TRUE;
}
GlibIOInputHandler::GlibIOInputHandler(
diff --git a/glib_io_ready_handler.cc b/glib_io_ready_handler.cc
index 985b225..7e1fc63 100644
--- a/glib_io_ready_handler.cc
+++ b/glib_io_ready_handler.cc
@@ -4,8 +4,10 @@
#include "shill/glib_io_ready_handler.h"
+#include <errno.h>
#include <glib.h>
#include <stdio.h>
+#include <string.h>
#include <sys/socket.h>
#include "shill/logging.h"
@@ -18,11 +20,13 @@
GIOCondition cond,
gpointer data) {
GlibIOReadyHandler *handler = reinterpret_cast<GlibIOReadyHandler *>(data);
-
- handler->callback().Run(g_io_channel_unix_get_fd(chan));
+ gint fd = g_io_channel_unix_get_fd(chan);
if (cond & (G_IO_NVAL | G_IO_HUP | G_IO_ERR))
- return FALSE;
+ LOG(FATAL) << "Unexpected GLib error condition " << cond << " on poll("
+ << fd << "): " << strerror(errno);
+
+ handler->callback().Run(fd);
return TRUE;
}