Bug 17866

Oh, actually that was not quite correct. ck-event-logger.c did change in 0.3, but we already have that patch backported (dfcab49480565a7bcf71752c5b39eb367df81a19, "cleanly shutdown event logging thread").

(I have test packages for 0.3 ready for current Ubuntu, together with the hal and packagekit fixes for the ABI change, but they still break things, so we can't upgrade to 0.3 yet.)

Ah, so that happens in check_file_stream(), here in particular if the file changed underneath:

        if (old_stats.st_ino != new_stats.st_ino || old_stats.st_dev != new_stats.st_dev) {
                g_debug ("File %s has been replaced; writing to end of new file", event_logger->priv->log_filename);
                reopen_file_stream (event_logger);

I just checked the code, and priv->file is only ever fdopen()'ed from priv->fd. Now man fdopen explains the crash: "The file descriptor is not dup’ed, and will be closed when the stream created by fdopen() is closed.".

Thus it seems this crash is due to close()ing priv->fd twice in reopen_file_stream():

  close (event_logger->priv->fd);
  fclose (event_logger->priv->file);

I confirmed that by instrumenting reopen_file_stream():

        g_debug ("Reopening %s", event_logger->priv->log_filename);
        close (event_logger->priv->fd);
        if (fclose (event_logger->priv->file) != 0)
            perror("fclose");

started the daemon, did a "rm /var/log/ConsoleKit/history; touch /var/log/ConsoleKit/history; ck-launch-session" and got

console-kit-daemon[15108]: DEBUG: Reopening /var/log/ConsoleKit/history
fclose: Bad file descriptor

Now I'm unclear why this actually causes a segfault at times, and not just a failed close() call, but that's something for the glibc developers to ask, I think.

In any case it is correct to remove the first close(). fclose() will flush the stream and call close() on priv->fd.

Created attachment 19338 [details] [review]
fix double close()

Looks good.  I've committed this to git.  Thanks!