MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:December 15 2010 1:26pm
Subject:bzr commit into mysql-5.1-bugteam branch (davi:3521) Bug#51023
View as plain text  
# At a local mysql-5.1-bugteam repository of davi

 3521 Davi Arnaut	2010-12-15
      Bug#51023: Mysql server crashes on SIGHUP and destroys InnoDB files
      
      From a user perspective, the problem is that a FLUSH LOGS or SIGHUP
      signal could end up associating the stdout and stderr to random
      files. In the case of this bug report, the streams would end up
      associated to InnoDB ibd files.
      
      The freopen(3) function is not thread-safe on FreeBSD. What this
      means is that if another thread calls open(2) during freopen()
      is executing that another thread's fd returned by open(2) may get
      re-associated with the file being passed to freopen(3). See FreeBSD
      PR number 79887 for reference:
      
        http://www.freebsd.org/cgi/query-pr.cgi?pr=79887
      
      This problem is worked around by substituting a internal hook within
      the FILE structure. This avoids the loss of atomicity by not having
      the original fd closed before its duplicated.
      
      Patch based on the original work by Vasil Dimov.
     @ include/my_sys.h
        Export my_freopen.
     @ mysys/my_fopen.c
        Add a my_freopen abstraction to workaround bugs in specific OSes.
     @ sql/log.cc
        Move freopen abstraction code over to mysys.
        The streams are now only reopened for writing.

    modified:
      include/my_sys.h
      mysys/my_fopen.c
      sql/log.cc
=== modified file 'include/my_sys.h'
--- a/include/my_sys.h	2010-06-08 21:14:18 +0000
+++ b/include/my_sys.h	2010-12-15 13:26:20 +0000
@@ -664,6 +664,7 @@ extern void init_glob_errs(void);
 extern void wait_for_free_space(const char *filename, int errors);
 extern FILE *my_fopen(const char *FileName,int Flags,myf MyFlags);
 extern FILE *my_fdopen(File Filedes,const char *name, int Flags,myf MyFlags);
+extern FILE *my_freopen(const char *path, const char *mode, FILE *stream);
 extern int my_fclose(FILE *fd,myf MyFlags);
 extern int my_chsize(File fd,my_off_t newlength, int filler, myf MyFlags);
 extern int my_sync(File fd, myf my_flags);

=== modified file 'mysys/my_fopen.c'
--- a/mysys/my_fopen.c	2007-08-13 13:11:25 +0000
+++ b/mysys/my_fopen.c	2010-12-15 13:26:20 +0000
@@ -18,6 +18,10 @@
 #include <errno.h>
 #include "mysys_err.h"
 
+#if defined(__FreeBSD__)
+extern int getosreldate(void);
+#endif
+
 static void make_ftype(char * to,int flag);
 
 /*
@@ -97,8 +101,137 @@ FILE *my_fopen(const char *filename, int
 } /* my_fopen */
 
 
-	/* Close a stream */
+#if defined(_WIN32)
+
+static FILE *my_win_freopen(const char *path, FILE *stream)
+{
+  int handle_fd, fd= _fileno(stream);
+  HANDLE osfh;
+
+  DBUG_ASSERT(filename && stream);
+
+  // Services don't have stdout/stderr on Windows, so _fileno returns -1.
+  if (fd < 0)
+  {
+    if (!freopen(filename, mode, stream))
+      return NULL;
+
+    fd= _fileno(stream);
+  }
+
+  if ((osfh= CreateFile(path, GENERIC_READ | GENERIC_WRITE,
+                        FILE_SHARE_READ | FILE_SHARE_WRITE |
+                        FILE_SHARE_DELETE, NULL,
+                        OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL,
+                        NULL)) == INVALID_HANDLE_VALUE)
+    return NULL;
+
+  if ((handle_fd= _open_osfhandle((intptr_t)osfh,
+                                  _O_APPEND | _O_TEXT)) == -1)
+  {
+    CloseHandle(osfh);
+    return NULL;
+  }
+
+  if (_dup2(handle_fd, fd) < 0)
+  {
+    CloseHandle(osfh);
+    return NULL;
+  }
+
+  _close(handle_fd);
+
+  return stream;
+}
+
+#elif defined(__FreeBSD__)
+
+/* No close operation hook. */
+
+static int no_close(void *cookie __attribute__((unused)))
+{
+  return 0;
+}
+
+/*
+  A hack around a race condition in the implementation of freopen.
+
+  The race condition steams from the fact that the current fd of
+  the stream is closed before its number is used to duplicate the
+  new file descriptor. This defeats the desired atomicity of the
+  close and duplicate of dup2().
+
+  See PR number 79887 for reference:
+  http://www.freebsd.org/cgi/query-pr.cgi?pr=79887
+*/
+
+static FILE *my_freebsd_freopen(const char *path, const char *mode, FILE *stream)
+{
+  int old_fd;
+  FILE *result;
+
+  flockfile(stream);
+
+  old_fd= fileno(stream);
+
+  /* Use a no operation close hook to avoid having the fd closed. */
+  stream->_close= no_close;
+
+  /* Relies on the implicit dup2 to close old_fd. */
+  result= freopen(path, mode, stream);
+
+  /* If successful, the _close hook was replaced. */
+
+  if (result == NULL)
+    close(old_fd);
+  else
+    funlockfile(result);
+
+  return result;
+}
+
+#endif
+
+
+/**
+  Change the file associated with a file stream.
+
+  @param path   Path to file.
+  @param mode   Mode of the stream.
+  @param stream File stream.
+
+  @note
+    This function is used to redirect stdout and stderr to a file and
+    subsequently to close and reopen that file for log rotation.
+
+  @retval A FILE pointer on success. Otherwise, NULL.
+*/
+
+FILE *my_freopen(const char *path, const char *mode, FILE *stream)
+{
+  FILE *result;
+
+#if defined(_WIN32)
+  result= my_win_freopen(path, mode, stream);
+#elif defined(__FreeBSD__)
+  /*
+    XXX: Once the fix is ported to the stable releases, this should
+         be dependent upon the specific FreeBSD versions. Check at:
+         http://www.freebsd.org/cgi/query-pr.cgi?pr=79887
+  */
+  if (getosreldate() > 900027)
+    result= freopen(path, mode, stream);
+  else
+    result= my_freebsd_freopen(path, mode, stream);
+#else
+  result= freopen(path, mode, stream);
+#endif
+
+  return result;
+}
+
 
+/* Close a stream */
 int my_fclose(FILE *fd, myf MyFlags)
 {
   int err,file;

=== modified file 'sql/log.cc'
--- a/sql/log.cc	2010-11-30 16:55:28 +0000
+++ b/sql/log.cc	2010-12-15 13:26:20 +0000
@@ -5067,80 +5067,26 @@ void sql_perror(const char *message)
 }
 
 
-#ifdef __WIN__
+/*
+  Change the file associated with two output streams. Used to
+  redirect stdout and stderr to a file. The streams are reopened
+  only for appending (writing at end of file).
+*/
 extern "C" my_bool reopen_fstreams(const char *filename,
                                    FILE *outstream, FILE *errstream)
 {
-  int handle_fd;
-  int err_fd, out_fd;
-  HANDLE osfh;
-
-  DBUG_ASSERT(filename && errstream);
-
-  // Services don't have stdout/stderr on Windows, so _fileno returns -1.
-  err_fd= _fileno(errstream);
-  if (err_fd < 0)
-  {
-    if (!freopen(filename, "a+", errstream))
-      return TRUE;
-
-    setbuf(errstream, NULL);
-    err_fd= _fileno(errstream);
-  }
-
-  if (outstream)
-  {
-    out_fd= _fileno(outstream);
-    if (out_fd < 0)
-    {
-      if (!freopen(filename, "a+", outstream))
-        return TRUE;
-      out_fd= _fileno(outstream);
-    }
-  }
-
-  if ((osfh= CreateFile(filename, GENERIC_READ | GENERIC_WRITE,
-                        FILE_SHARE_READ | FILE_SHARE_WRITE |
-                        FILE_SHARE_DELETE, NULL,
-                        OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL,
-                        NULL)) == INVALID_HANDLE_VALUE)
+  if (outstream && !my_freopen(filename, "a", outstream))
     return TRUE;
 
-  if ((handle_fd= _open_osfhandle((intptr_t)osfh,
-                                  _O_APPEND | _O_TEXT)) == -1)
-  {
-    CloseHandle(osfh);
+  if (errstream && !my_freopen(filename, "a", errstream))
     return TRUE;
-  }
 
-  if (_dup2(handle_fd, err_fd) < 0)
-  {
-    CloseHandle(osfh);
-    return TRUE;
-  }
-
-  if (outstream && _dup2(handle_fd, out_fd) < 0)
-  {
-    CloseHandle(osfh);
-    return TRUE;
-  }
-
-  _close(handle_fd);
-  return FALSE;
-}
-#else
-extern "C" my_bool reopen_fstreams(const char *filename,
-                                   FILE *outstream, FILE *errstream)
-{
-  if (outstream && !freopen(filename, "a+", outstream))
-    return TRUE;
-
-  if (errstream && !freopen(filename, "a+", errstream))
-    return TRUE;
+  /* The error stream must be unbuffered. */
+  if (errstream)
+    setbuf(errstream, NULL);
 
   return FALSE;
 }
-#endif
 
 
 /*


Attachment: [text/bzr-bundle] bzr/davi.arnaut@oracle.com-20101215132620-zef4gx3862ojho04.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (davi:3521) Bug#51023Davi Arnaut15 Dec