Bug#30183865: HUGE MALLOC WHEN OPEN FILE LIMIT IS HIGH

Problem: Setting max_open_files to a high value, or setting it when
the OS' rlimit had a large value (not equal to RLIM_INF) could cause
the server run out of memory.

Root cause was that the server would allocate an a array with capacity
equal to the max_open_files setting at startup, and in particular that
the OS rlim setting would override the setting supplied by the user,
even if that value was much larger.

Solution: A simple solution would be to not let the OS override the
setting provided by the user. But this is a change in behavior which
could potentially break existing applications. Moreover, it would not
protect against the user setting the limit to an excessively large
value. Furthermore, allocating space for the largest possible number of
open files, is a pessimisation, since it is unlikely that all those
array slots will be needed.

In this patch the issue is addressed by changing the array into a
vector and letting it grow as needed when files are opened.

Additionally, on Windows the same file_info array was used to provide
a mapping between Windows HANDLEs and pseudo file descriptors so that
the mysys file functions could use integers to refer to files also on
Windows. In order to hide this functionality in Windows-specific code
and reduce the number of #ifdefs this mapping was moved into its own
vector and data structure. In the process the mysys functions
emulating posix functions with Windows API functions was re-factored
and error handling tightened. The latter was necessary since negative
testing using invalid pseudo file descriptors had to be intercepted
earlier to avoid stepping outside the bounds of the vector mapping
pseudo fds to HANDLEs.

Change-Id: Ib255a509e8c538d79f316d642165263609961e78

Author: Dyre Tjeldvoll

client/mysqlbinlog.cc

@@ -2868,7 +2868,6 @@ int main(int argc, char **argv) {
   if (result_file && (result_file != stdout)) my_fclose(result_file, MYF(0));
   cleanup();
 
-  my_free_open_file_info();
   load_processor.destroy();
   /* We cannot free DBUG, it is used in global destructors after exit(). */
   my_end(my_end_arg | MY_DONT_FREE_DBUG);

client/mysqltest.cc

@@ -7674,7 +7674,7 @@ void str_to_file2(const char *fname, char *str, size_t size, bool append) {
   fn_format(buff, fname, "", "", MY_UNPACK_FILENAME);
 
   if (!append) flags |= O_TRUNC;
-  if ((fd = my_open(buff, flags, MYF(MY_WME | MY_FFNF))) < 0)
+  if ((fd = my_open(buff, flags, MYF(MY_WME))) < 0)
     die("Could not open '%s' for writing, errno: %d", buff, errno);
   if (append && my_seek(fd, 0, SEEK_END, MYF(0)) == MY_FILEPOS_ERROR)
     die("Could not find end of file '%s', errno: %d", buff, errno);

config.h.cmake

@@ -97,7 +97,6 @@
 #cmakedefine HAVE_GETPASSPHRASE 1
 #cmakedefine HAVE_GETPWNAM 1
 #cmakedefine HAVE_GETPWUID 1
-#cmakedefine HAVE_GETRLIMIT 1
 #cmakedefine HAVE_GETRUSAGE 1
 #cmakedefine HAVE_INITGROUPS 1
 #cmakedefine HAVE_ISSETUGID 1
@@ -316,6 +315,7 @@
 /*
  * NDB
  */
+#cmakedefine HAVE_GETRLIMIT 1
 #cmakedefine WITH_NDBCLUSTER_STORAGE_ENGINE 1
 #cmakedefine HAVE_PTHREAD_SETSCHEDPARAM 1
 

configure.cmake

@@ -276,7 +276,6 @@ CHECK_FUNCTION_EXISTS (getpass HAVE_GETPASS)
 CHECK_FUNCTION_EXISTS (getpassphrase HAVE_GETPASSPHRASE)
 CHECK_FUNCTION_EXISTS (getpwnam HAVE_GETPWNAM)
 CHECK_FUNCTION_EXISTS (getpwuid HAVE_GETPWUID)
-CHECK_FUNCTION_EXISTS (getrlimit HAVE_GETRLIMIT)
 CHECK_FUNCTION_EXISTS (getrusage HAVE_GETRUSAGE)
 CHECK_FUNCTION_EXISTS (initgroups HAVE_INITGROUPS)
 CHECK_FUNCTION_EXISTS (issetugid HAVE_ISSETUGID)

include/my_dbug.h

@@ -328,6 +328,24 @@ extern void _db_flush_gcov_();
   do {                       \
   } while (0)
 #endif /* DBUG_OFF */
-#endif /* __cplusplus */
 
+/**
+   A type-safe interface to DBUG_EXECUTE_IF, where the debug action to
+   activate when the keyword is provided is given as a callable object
+   (typically a lambda).
+
+   @note The body of the callable will be checked by the compiler even
+         in optimized mode.
+
+   @param keyword String literal which will enable this debug action.
+   @param clos    Callable object taking no arguments which will be
+                  called in debug mode if the keyword is enabled.
+ */
+template <class DBGCLOS>
+inline void dbug(const char *keyword MY_ATTRIBUTE((unused)),
+                 DBGCLOS &&clos MY_ATTRIBUTE((unused))) {
+  DBUG_EXECUTE_IF(keyword, clos(););
+}
+
+#endif /* __cplusplus */
 #endif /* MY_DBUG_INCLUDED */

include/my_sys.h

@@ -109,7 +109,7 @@ struct MEM_ROOT;
 #define MY_FILE_ERROR ((size_t)-1)
 
 /* General bitmaps for my_func's */
-#define MY_FFNF 1          /* Fatal if file not found */
+// 1 used to be MY_FFNF which has been removed
 #define MY_FNABP 2         /* Fatal if not all bytes read/writen */
 #define MY_NABP 4          /* Error if not all bytes read/writen */
 #define MY_FAE 8           /* Fatal if any error */
@@ -189,7 +189,9 @@ extern PSI_memory_key key_memory_max_alloca;
   if (size > max_alloca_sz) my_free(ptr)
 
 #if defined(ENABLED_DEBUG_SYNC)
-extern "C" void (*debug_sync_C_callback_ptr)(const char *, size_t);
+using DebugSyncCallbackFp = void (*)(const char *, size_t);
+extern DebugSyncCallbackFp debug_sync_C_callback_ptr;
+
 #define DEBUG_SYNC_C(_sync_point_name_)                                 \
   do {                                                                  \
     if (debug_sync_C_callback_ptr != NULL)                              \
@@ -219,7 +221,7 @@ extern void (*error_handler_hook)(uint my_err, const char *str, myf MyFlags);
 extern void (*fatal_error_handler_hook)(uint my_err, const char *str,
                                         myf MyFlags);
 extern void (*local_message_hook)(enum loglevel ll, uint ecode, va_list args);
-extern uint my_file_limit;
+
 extern MYSQL_PLUGIN_IMPORT ulong my_thread_stack_size;
 
 /*
@@ -260,7 +262,9 @@ extern MYSQL_PLUGIN_IMPORT CHARSET_INFO *all_charsets[MY_ALL_CHARSETS_SIZE];
 extern CHARSET_INFO compiled_charsets[];
 
 /* statistics */
-extern ulong my_file_opened, my_stream_opened, my_tmp_file_created;
+extern ulong my_tmp_file_created;
+extern ulong my_file_opened;
+extern ulong my_stream_opened;
 extern ulong my_file_total_opened;
 extern bool my_init_done;
 
@@ -293,31 +297,6 @@ enum flush_type {
   FLUSH_FORCE_WRITE
 };
 
-/*
- How was this file opened (for debugging purposes).
- The important part is whether it is UNOPEN or not.
- */
-enum file_type {
-  UNOPEN = 0,
-  FILE_BY_OPEN,
-  FILE_BY_CREATE,
-  STREAM_BY_FOPEN,
-  STREAM_BY_FDOPEN,
-  FILE_BY_MKSTEMP,
-  FILE_BY_O_TMPFILE
-};
-
-struct st_my_file_info {
-  char *name;
-#ifdef _WIN32
-  HANDLE fhandle; /* win32 file handle */
-  int oflag;      /* open flags, e.g O_APPEND */
-#endif
-  enum file_type type;
-};
-
-extern struct st_my_file_info *my_file_info;
-
 struct DYNAMIC_ARRAY {
   uchar *buffer{nullptr};
   uint elements{0}, max_element{0};
@@ -584,13 +563,11 @@ extern void *my_once_alloc(size_t Size, myf MyFlags);
 extern void my_once_free(void);
 extern char *my_once_strdup(const char *src, myf myflags);
 extern void *my_once_memdup(const void *src, size_t len, myf myflags);
-extern File my_open(const char *FileName, int Flags, myf MyFlags);
-extern File my_register_filename(File fd, const char *FileName,
-                                 enum file_type type_of_file,
-                                 uint error_message_number, myf MyFlags);
+extern File my_open(const char *filename, int Flags, myf MyFlags);
+
 extern File my_create(const char *FileName, int CreateFlags, int AccessFlags,
                       myf MyFlags);
-extern int my_close(File Filedes, myf MyFlags);
+extern int my_close(File fd, myf MyFlags);
 extern int my_mkdir(const char *dir, int Flags, myf MyFlags);
 extern int my_readlink(char *to, const char *filename, myf MyFlags);
 extern int my_is_symlink(const char *filename, ST_FILE_ID *file_id);
@@ -665,11 +642,11 @@ extern void my_osmaperr(unsigned long last_error);
 
 extern const char *get_global_errmsg(int nr);
 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 File my_fileno(FILE *fd);
+extern FILE *my_fopen(const char *filename, int Flags, myf MyFlags);
+extern FILE *my_fdopen(File fd, const char *filename, int Flags, myf MyFlags);
+extern FILE *my_freopen(const char *filename, const char *mode, FILE *stream);
+extern int my_fclose(FILE *stream, myf MyFlags);
+extern File my_fileno(FILE *stream);
 extern int my_chsize(File fd, my_off_t newlength, int filler, myf MyFlags);
 extern int my_fallocator(File fd, my_off_t newlength, int filler, myf MyFlags);
 extern void thr_set_sync_wait_callback(void (*before_sync)(void),
@@ -689,18 +666,31 @@ extern void my_message(uint my_err, const char *str, myf MyFlags);
 extern void my_message_stderr(uint my_err, const char *str, myf MyFlags);
 void my_message_local_stderr(enum loglevel, uint ecode, va_list args);
 extern void my_message_local(enum loglevel ll, uint ecode, ...);
+
+/**
+  Convenience wrapper for OS error messages which report
+  errno/my_errno with %d followed by strerror as %s, as the last
+  conversions in the error message.
+
+  The OS error message (my_errno) is formatted in a stack buffer and
+  the errno value and a pointer to the buffer is added to the end of
+  the parameter pack passed to my_error().
+
+  @param errno_val errno/my_errno number.
+  @param ppck parameter pack of additional arguments to pass to my_error().
+ */
+template <class... Ts>
+inline void MyOsError(int errno_val, Ts... ppck) {
+  char errbuf[MYSYS_STRERROR_SIZE];
+  my_error(ppck..., errno_val, my_strerror(errbuf, sizeof(errbuf), errno_val));
+}
+
 extern bool my_init(void);
 extern void my_end(int infoflag);
 extern const char *my_filename(File fd);
 extern MY_MODE get_file_perm(ulong perm_flags);
 extern bool my_chmod(const char *filename, ulong perm_flags, myf my_flags);
 
-#ifdef EXTRA_DEBUG
-void my_print_open_files(void);
-#else
-#define my_print_open_files()
-#endif
-
 extern bool init_tmpdir(MY_TMPDIR *tmpdir, const char *pathlist);
 extern char *my_tmpdir(MY_TMPDIR *tmpdir);
 extern void free_tmpdir(MY_TMPDIR *tmpdir);
@@ -817,7 +807,6 @@ extern uchar *my_compress_alloc(mysql_compress_context *comp_ctx,
 extern ha_checksum my_checksum(ha_checksum crc, const uchar *mem, size_t count);
 
 extern uint my_set_max_open_files(uint files);
-void my_free_open_file_info(void);
 
 extern bool my_gethwaddr(uchar *to);
 

mysys/CMakeLists.txt

@@ -71,7 +71,6 @@ SET(MYSYS_SOURCES
   my_copy.cc
   my_create.cc
   my_delete.cc
-  my_div.cc
   my_error.cc
   my_fallocator.cc
   my_file.cc

mysys/mf_tempfile.cc

@@ -86,7 +86,8 @@
 
 File create_temp_file(char *to, const char *dir, const char *prefix,
                       int mode MY_ATTRIBUTE((unused)),
-                      UnlinkOrKeepFile unlink_or_keep, myf MyFlags) {
+                      UnlinkOrKeepFile unlink_or_keep,
+                      myf MyFlags MY_ATTRIBUTE((unused))) {
   File file = -1;
 #ifdef _WIN32
   TCHAR path_buf[MAX_PATH - 14];
@@ -149,19 +150,24 @@ File create_temp_file(char *to, const char *dir, const char *prefix,
     /* Explicitly don't use O_EXCL here as it has a different
        meaning with O_TMPFILE.
     */
-    file = open(dirname_buf, O_RDWR | O_TMPFILE | O_CLOEXEC, S_IRUSR | S_IWUSR);
+    file = mysys_priv::RetryOnEintr(
+        [&]() {
+          return open(dirname_buf, O_RDWR | O_TMPFILE | O_CLOEXEC,
+                      S_IRUSR | S_IWUSR);
+        },
+        -1);
+
     if (file >= 0) {
       sprintf(to, "%s%.20sfd=%d", dirname_buf, prefix ? prefix : "tmp.", file);
-      file = my_register_filename(file, to, FILE_BY_O_TMPFILE,
-                                  EE_CANTCREATEFILE, MyFlags);
+      file_info::RegisterFilename(file, to,
+                                  file_info::OpenType::FILE_BY_O_TMPFILE);
     }
   }
   // Fall through, in case open() failed above (or we have KEEP_FILE).
 #endif /* HAVE_O_TMPFILE */
   if (file == -1) {
     char prefix_buff[30];
     uint pfx_len;
-    File org_file;
 
     pfx_len = (uint)(my_stpcpy(my_stpnmov(prefix_buff, prefix ? prefix : "tmp.",
                                           sizeof(prefix_buff) - 7),
@@ -174,22 +180,13 @@ File create_temp_file(char *to, const char *dir, const char *prefix,
       return file;
     }
     my_stpcpy(convert_dirname(to, dir, NullS), prefix_buff);
-    org_file = mkstemp(to);
-    file = my_register_filename(org_file, to, FILE_BY_MKSTEMP,
-                                EE_CANTCREATEFILE, MyFlags);
-    /* If we didn't manage to register the name, remove the temp file */
-    if (org_file >= 0 && file < 0) {
-      int tmp = my_errno();
-      close(org_file);
-      (void)my_delete(to, MYF(MY_WME));
-      set_my_errno(tmp);
-      return file;
-    }
+    file = mkstemp(to);
+    file_info::RegisterFilename(file, to, file_info::OpenType::FILE_BY_MKSTEMP);
     if (unlink_or_keep == UNLINK_FILE) {
       unlink(to);
     }
   }
-#endif
+#endif /* _WIN32 */
   if (file >= 0) {
     mysql_mutex_lock(&THR_LOCK_open);
     my_tmp_file_created++;

mysys/my_chmod.cc

@@ -85,7 +85,7 @@ MY_MODE get_file_perm(ulong perm_flags) {
     @retval false : File permission changed successfully
 */
 
-bool my_chmod(const char *filename, ulong perm_flags, myf my_flags) {
+bool my_chmod(const char *filename, ulong perm_flags, myf MyFlags) {
   int ret_val;
   MY_MODE file_perm;
   DBUG_TRACE;
@@ -98,11 +98,9 @@ bool my_chmod(const char *filename, ulong perm_flags, myf my_flags) {
   ret_val = chmod(filename, file_perm);
 #endif
 
-  if (ret_val && (my_flags & (MY_FAE + MY_WME))) {
-    char errbuf[MYSYS_STRERROR_SIZE];
+  if (ret_val && (MyFlags & (MY_FAE | MY_WME))) {
     set_my_errno(errno);
-    my_error(EE_CHANGE_PERMISSIONS, MYF(0), filename, errno,
-             my_strerror(errbuf, sizeof(errbuf), errno));
+    MyOsError(my_errno(), EE_CHANGE_PERMISSIONS, MYF(0), filename);
   }
 
   return ret_val ? true : false;

mysys/my_chsize.cc

@@ -108,9 +108,7 @@ int my_chsize(File fd, my_off_t newlength, int filler, myf MyFlags) {
 
 err:
   if (MyFlags & MY_WME) {
-    char errbuf[MYSYS_STRERROR_SIZE];
-    my_error(EE_CANT_CHSIZE, MYF(0), my_errno(),
-             my_strerror(errbuf, sizeof(errbuf), my_errno()));
+    MyOsError(my_errno(), EE_CANT_CHSIZE, MYF(0));
   }
   return 1;
 }