From: VMware, Inc <> Date: Mon, 21 Nov 2011 23:19:26 +0000 (-0800) Subject: glib logger: don't try to log to an invalid fd. X-Git-Tag: 2011.11.20-535097~48 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=5904b29ab781eca8c7904de86d70451be34da554;p=thirdparty%2Fopen-vm-tools.git glib logger: don't try to log to an invalid fd. See nasty details in the comment for FileLoggerIsValid(). Long story short, glib will cause log recursion and because it abort(3)s the process when that happens instead of letting us handle the recursion, things don't work. Signed-off-by: Marcelo Vanzin --- diff --git a/open-vm-tools/lib/glibUtils/fileLogger.c b/open-vm-tools/lib/glibUtils/fileLogger.c index ffcc7f83f..14df1c8f5 100644 --- a/open-vm-tools/lib/glibUtils/fileLogger.c +++ b/open-vm-tools/lib/glibUtils/fileLogger.c @@ -30,6 +30,7 @@ # include # include #else +# include # include #endif @@ -47,6 +48,56 @@ typedef struct FileLogger { } FileLogger; +#if !defined(_WIN32) +/* + ******************************************************************************* + * FileLoggerIsValid -- */ /** + * + * Checks that the file descriptor backing this logger is still valid. + * + * This is a racy workaround for an issue with glib code; or, rather, two + * issues. The first issue is that we can't intercept G_LOG_FLAG_RECURSION, + * and glib just aborts when that happens (see gnome bug 618956). The second + * is that if a GIOChannel channel write fails, that calls + * g_io_channel_error_from_errno, which helpfully logs stuff, causing recursion. + * Don't get me started on why that's, well, at least questionable. + * + * This is racy because between the check and the actual GIOChannel operation, + * the state of the FD may have changed. In reality, since the bug this is + * fixing happens in very special situations where code outside this file is + * doing weird things like closing random fds, it should be OK. + * + * We may still get other write errors from the GIOChannel than EBADF, but + * those would be harder to work around. Hopefully this handles the most usual + * cases. + * + * See bug 783999 for some details about what triggers the bug. + * + * @param[in] logger The logger instance. + * + * @return TRUE if the I/O channel is still valid. + * + ******************************************************************************* + */ + +static gboolean +FileLoggerIsValid(FileLogger *logger) +{ + if (logger->file != NULL) { + int fd = g_io_channel_unix_get_fd(logger->file); + return fcntl(fd, F_GETFD) >= 0; + } + + return FALSE; +} + +#else + +#define FileLoggerIsValid(logger) TRUE + +#endif + + /* ******************************************************************************* * FileLoggerGetPath -- */ /** @@ -268,6 +319,11 @@ FileLoggerLog(const gchar *domain, } } + if (!FileLoggerIsValid(logger)) { + logger->error = TRUE; + goto exit; + } + /* Write the log file and do log rotation accounting. */ if (g_io_channel_write_chars(logger->file, message, -1, &written, NULL) == G_IO_STATUS_NORMAL) {