Various fixes (cleanups, valgrind, makefiles, ...)

server-tools/instance-manager/Makefile.am:
  increased default_monitoring interval
server-tools/instance-manager/guardian.cc:
  some fixes for proper shutdown
server-tools/instance-manager/guardian.h:
  removed init() prototype, as it was never used
server-tools/instance-manager/instance.cc:
  cleanup() function removed
server-tools/instance-manager/instance.h:
  cleanup() prototype removed
server-tools/instance-manager/instance_map.cc:
  Instance_map::cleanup() removed, as instances have nothing to clean up
server-tools/instance-manager/instance_map.h:
  Instance_map::cleanup() prototype removed
server-tools/instance-manager/instance_options.cc:
  added print_argv() function for debug purposes
server-tools/instance-manager/instance_options.h:
  declared print_argv()
server-tools/instance-manager/listener.cc:
  some fixed in listener for proper shutdown
server-tools/instance-manager/log.cc:
  cleanup
server-tools/instance-manager/log.h:
  cleanup
server-tools/instance-manager/manager.cc:
  some comments added
server-tools/instance-manager/mysqlmanager.cc:
  we need to free memory in the very end
server-tools/instance-manager/options.cc:
  fixed default options handling (as they were not working properly), added new method to cleanup Options
server-tools/instance-manager/options.h:
  cleanup() declared
server-tools/instance-manager/thread_registry.cc:
  cleanup
server-tools/instance-manager/user_map.cc:
  missing password file is not a critical error anymore, as IM should be able to work as mysqld_safe only

server-tools/instance-manager/Makefile.am

@@ -33,7 +33,7 @@ liboptions_a_CPPFLAGS= $(CPPFLAGS) \
 	-DDEFAULT_MYSQLD_PATH="$(bindir)/mysqld$(EXEEXT)" \
 	-DDEFAULT_USER="root" \
 	-DDEFAULT_PASSWORD="" \
-	-DDEFAULT_MONITORING_INTERVAL="5" \
+	-DDEFAULT_MONITORING_INTERVAL="20" \
 	-DDEFAULT_PORT="2273" \
 	-DPROTOCOL_VERSION=@PROTOCOL_VERSION@
 
@@ -59,7 +59,7 @@ client_settings.h: Makefile
 	rm -f $(srcdir)/client_settings.h
 	@LN_CP_F@ $(top_srcdir)/sql/client_settings.h $(srcdir)/client_settings.h
 
-bin_PROGRAMS= mysqlmanager
+libexec_PROGRAMS= mysqlmanager
 
 mysqlmanager_SOURCES=	command.cc command.h mysqlmanager.cc \
 			manager.h manager.cc log.h log.cc \

server-tools/instance-manager/guardian.cc

@@ -48,7 +48,6 @@ Guardian_thread::Guardian_thread(Thread_registry &thread_registry_arg,
   pthread_cond_init(&COND_guardian, 0);
   shutdown_guardian= FALSE;
   is_stopped= FALSE;
-  thread_registry.register_thread(&thread_info);
   init_alloc_root(&alloc, MEM_ROOT_BLOCK_SIZE, 0);
   guarded_instances= NULL;
   starting_instances= NULL;
@@ -60,7 +59,6 @@ Guardian_thread::~Guardian_thread()
   /* delay guardian destruction to the moment when no one needs it */
   pthread_mutex_lock(&LOCK_guardian);
   free_root(&alloc, MYF(0));
-  thread_registry.unregister_thread(&thread_info);
   pthread_mutex_unlock(&LOCK_guardian);
   pthread_mutex_destroy(&LOCK_guardian);
   pthread_cond_destroy(&COND_guardian);
@@ -102,6 +100,8 @@ void Guardian_thread::run()
   LIST *loop;
   struct timespec timeout;
 
+  thread_registry.register_thread(&thread_info);
+
   my_thread_init();
   pthread_mutex_lock(&LOCK_guardian);
 
@@ -110,6 +110,7 @@ void Guardian_thread::run()
   {
     int status= 0;
     loop= guarded_instances;
+
     while (loop != NULL)
     {
       instance= ((GUARD_NODE *) loop->data)->instance;
@@ -167,6 +168,7 @@ void Guardian_thread::run()
     stop_instances();
   is_stopped= TRUE;
   /* now, when the Guardian is stopped we can stop the IM */
+  thread_registry.unregister_thread(&thread_info);
   thread_registry.request_shutdown();
   my_thread_end();
 }
@@ -181,7 +183,7 @@ int Guardian_thread::start()
   while ((instance= iterator.next()))
   {
     if ((instance->options.nonguarded == NULL))
-      if (guard(instance))
+      if (add_instance_to_list(instance, &guarded_instances))
         return 1;
   }
   instance_map->unlock();

server-tools/instance-manager/guardian.h

@@ -73,7 +73,6 @@ class Guardian_thread: public Guardian_thread_args
                   uint monitoring_interval_arg);
   ~Guardian_thread();
   void run();
-  int init();
   int start();
   void shutdown();
   void request_stop_instances();

server-tools/instance-manager/instance.cc

@@ -56,6 +56,7 @@ int Instance::start()
     switch (pid= fork()) {
     case 0:
       execv(options.mysqld_path, options.argv);
+      /* exec never returns */
       exit(1);
     case -1:
       return ER_CANNOT_START_INSTANCE;
@@ -69,12 +70,6 @@ int Instance::start()
 }
 
 
-int Instance::cleanup()
-{
-  return 0;
-}
-
-
 Instance::~Instance()
 {
   pthread_mutex_destroy(&LOCK_instance);

server-tools/instance-manager/instance.h

@@ -38,7 +38,6 @@ class Instance
   bool is_running();
   int start();
   int stop();
-  int cleanup();
 
 public:
   enum { DEFAULT_SHUTDOWN_DELAY= 35 };

server-tools/instance-manager/instance_map.cc

@@ -198,23 +198,6 @@ void Instance_map::complete_initialization()
 }
 
 
-int Instance_map::cleanup()
-{
-  Instance *instance;
-  uint i= 0;
-
-  while (i < hash.records)
-  {
-    instance= (Instance *) hash_element(&hash, i);
-    if (instance->cleanup())
-			return 1;
-    i++;
-  }
-
-	return 0;
-}
-
-
 Instance *
 Instance_map::find(uint instance_number)
 {

server-tools/instance-manager/instance_map.h

@@ -66,7 +66,6 @@ class Instance_map
   Instance *find(uint instance_number);
 
   int flush_instances();
-  int cleanup();
   int lock();
   int unlock();
   int init();

server-tools/instance-manager/instance_options.cc

@@ -241,6 +241,16 @@ int Instance_options::add_to_argv(const char* option)
   return 0;
 }
 
+void Instance_options::print_argv()
+{
+  int i;
+  printf("printing out an instance %s argv:\n", instance_name);
+  for (i=0; argv[i] != NULL; i++)
+  {
+    printf("argv: %s\n", argv[i]);
+  }
+}
+
 
 /*
   We execute this function to initialize some options.

server-tools/instance-manager/instance_options.h

@@ -50,6 +50,7 @@ class Instance_options
   pid_t get_pid();
   void get_pid_filename(char *result);
   int unlink_pidfile();
+  void print_argv();
 
 public:
   /*

server-tools/instance-manager/listener.cc

@@ -57,13 +57,11 @@ Listener_thread::Listener_thread(const Listener_thread_args &args) :
   ,total_connection_count(0)
   ,thread_info(pthread_self())
 {
-  thread_registry.register_thread(&thread_info);
 }
 
 
 Listener_thread::~Listener_thread()
 {
-  thread_registry.unregister_thread(&thread_info);
 }
 
 
@@ -82,6 +80,11 @@ void Listener_thread::run()
   enum { LISTEN_BACK_LOG_SIZE = 5 };            // standard backlog size
   int flags;
   int arg= 1;                             /* value to be set by setsockopt */
+  int unix_socket;
+  uint im_port;
+
+  thread_registry.register_thread(&thread_info);
+
   /* I. prepare 'listen' sockets */
 
   int ip_socket= socket(AF_INET, SOCK_STREAM, 0);
@@ -89,8 +92,7 @@ void Listener_thread::run()
   {
     log_error("Listener_thead::run(): socket(AF_INET) failed, %s",
               strerror(errno));
-    thread_registry.request_shutdown();
-    return;
+    goto err;
   }
 
   struct sockaddr_in ip_socket_address;
@@ -104,7 +106,7 @@ void Listener_thread::run()
   }
   else
     im_bind_addr= htonl(INADDR_ANY);
-  uint im_port= options.port_number;
+  im_port= options.port_number;
 
   ip_socket_address.sin_family= AF_INET;
   ip_socket_address.sin_addr.s_addr = im_bind_addr;
@@ -119,16 +121,14 @@ void Listener_thread::run()
   {
     log_error("Listener_thread::run(): bind(ip socket) failed, '%s'",
               strerror(errno));
-    thread_registry.request_shutdown();
-    return;
+   goto err;
   }
 
   if (listen(ip_socket, LISTEN_BACK_LOG_SIZE))
   {
     log_error("Listener_thread::run(): listen(ip socket) failed, %s",
               strerror(errno));
-    thread_registry.request_shutdown();
-    return;
+    goto err;
   }
       /* set the socket nonblocking */
   flags= fcntl(ip_socket, F_GETFL, 0);
@@ -140,13 +140,12 @@ void Listener_thread::run()
   log_info("accepting connections on ip socket");
 
   /*--------------------------------------------------------------*/
-  int unix_socket= socket(AF_UNIX, SOCK_STREAM, 0);
+  unix_socket= socket(AF_UNIX, SOCK_STREAM, 0);
   if (unix_socket == INVALID_SOCKET)
   {
     log_error("Listener_thead::run(): socket(AF_UNIX) failed, %s",
               strerror(errno));
-    thread_registry.request_shutdown();
-    return;
+    goto err;
   }
 
   struct sockaddr_un unix_socket_address;
@@ -169,8 +168,7 @@ void Listener_thread::run()
       log_error("Listener_thread::run(): bind(unix socket) failed, "
                 "socket file name is '%s', error '%s'",
                 unix_socket_address.sun_path, strerror(errno));
-      thread_registry.request_shutdown();
-      return;
+      goto err;
     }
     umask(old_mask);
 
@@ -178,8 +176,7 @@ void Listener_thread::run()
     {
       log_error("Listener_thread::run(): listen(unix socket) failed, %s",
                 strerror(errno));
-      thread_registry.request_shutdown();
-      return;
+      goto err;
     }
 
       /* set the socket nonblocking */
@@ -205,7 +202,15 @@ void Listener_thread::run()
     while (thread_registry.is_shutdown() == false)
     {
       fd_set read_fds_arg= read_fds;
+
+      /*
+        When using valgrind 2.0 this syscall doesn't get kicked off by a
+        signal during shutdown. This results in failing assert
+        (Thread_registry::~Thread_registry). Valgrind 2.2 works fine.
+      */
       int rc= select(n, &read_fds_arg, 0, 0, 0);
+
+
       if (rc == -1 && errno != EINTR)
         log_error("Listener_thread::run(): select() failed, %s",
                   strerror(errno));
@@ -256,6 +261,14 @@ void Listener_thread::run()
   close(unix_socket);
   close(ip_socket);
   unlink(unix_socket_address.sun_path);
+
+  thread_registry.unregister_thread(&thread_info);
+  return;
+
+err:
+  thread_registry.unregister_thread(&thread_info);
+  thread_registry.request_shutdown();
+  return;
 }
 
 

server-tools/instance-manager/manager.cc

@@ -127,6 +127,12 @@ void manager(const Options &options)
     pthread_attr_t guardian_thd_attr;
     int rc;
 
+    /*
+       NOTE: Guardian should be shutdowned first. Only then all other threads
+       need to be stopped. This should be done, as guardian is responsible for
+       shutting down the instances, and this is a long operation.
+    */
+
     pthread_attr_init(&guardian_thd_attr);
     pthread_attr_setdetachstate(&guardian_thd_attr, PTHREAD_CREATE_DETACHED);
     rc= pthread_create(&guardian_thd_id, &guardian_thd_attr, guardian,
@@ -160,6 +166,11 @@ void manager(const Options &options)
     making the list. And they in their turn need alarms for timeout suppport.
   */
   guardian_thread.start();
+  /*
+    After the list of guarded instances have been initialized,
+    Guardian should start them.
+  */
+  pthread_cond_signal(&guardian_thread.COND_guardian);
 
   signal(SIGPIPE, SIG_IGN);
 
@@ -176,14 +187,14 @@ void manager(const Options &options)
         /* wake threads waiting for an instance to shutdown */
         pthread_cond_broadcast(&instance_map.pid_cond.COND_pid);
         /* wake guardian */
-        pthread_cond_broadcast(&guardian_thread.COND_guardian);
+        pthread_cond_signal(&guardian_thread.COND_guardian);
       break;
       default:
         if (!guardian_thread.is_stopped)
         {
           guardian_thread.request_stop_instances();
           guardian_thread.shutdown();
-          pthread_cond_broadcast(&guardian_thread.COND_guardian);
+          pthread_cond_signal(&guardian_thread.COND_guardian);
         }
         else
         {
@@ -198,9 +209,6 @@ void manager(const Options &options)
   /* delete the pid file */
   my_delete(options.pid_file_name, MYF(0));
 
-  /* close permanent connections to the running instances */
-  instance_map.cleanup();
-
   /* free alarm structures */
   end_thr_alarm(1);
   /* don't pthread_exit to kill all threads who did not shut down in time */

server-tools/instance-manager/mysqlmanager.cc

@@ -77,6 +77,8 @@ int main(int argc, char *argv[])
     angel(options);
   }
   manager(options);
+  options.cleanup();
+  my_end(0);
   return 0;
 }
 

server-tools/instance-manager/options.cc

@@ -38,6 +38,8 @@ const char *Options::default_mysqld_path= QUOTE(DEFAULT_MYSQLD_PATH);
 const char *Options::bind_address= 0;              /* No default value */
 uint Options::monitoring_interval= DEFAULT_MONITORING_INTERVAL;
 uint Options::port_number= DEFAULT_PORT;
+/* just to declare */
+char **Options::saved_argv;
 
 /*
   List of options, accepted by the instance manager.
@@ -81,7 +83,7 @@ static struct my_option my_long_options[] =
 
   { "port", OPT_PORT, "Port number to use for connections",
     (gptr *) &Options::port_number, (gptr *) &Options::port_number,
-    0, GET_UINT, REQUIRED_ARG, 0, 0, 0, 0, 0, 0 },
+    0, GET_UINT, REQUIRED_ARG, DEFAULT_PORT, 0, 0, 0, 0, 0 },
 
   { "password-file", OPT_PASSWORD_FILE, "Look for Instane Manager users"
                                         " and passwords here.",
@@ -98,7 +100,8 @@ static struct my_option my_long_options[] =
                                             " in seconds.",
                    (gptr *) &Options::monitoring_interval,
                    (gptr *) &Options::monitoring_interval,
-                   0, GET_UINT, REQUIRED_ARG, 0, 0, 0, 0, 0, 0 },
+                   0, GET_UINT, REQUIRED_ARG, DEFAULT_MONITORING_INTERVAL,
+                   0, 0, 0, 0, 0 },
 
   { "run-as-service", OPT_RUN_AS_SERVICE,
     "Daemonize and start angel process.", (gptr *) &Options::run_as_service,
@@ -171,5 +174,11 @@ void Options::load(int argc, char **argv)
 
   if (int rc= handle_options(&argc, &argv, my_long_options, get_one_option))
     exit(rc);
+  Options::saved_argv= argv;
 }
 
+void Options::cleanup()
+{
+  /* free_defaults returns nothing */
+  free_defaults(Options::saved_argv);
+}

server-tools/instance-manager/options.h

@@ -40,7 +40,10 @@ struct Options
   static uint port_number;
   static const char *bind_address;
 
+  static char **saved_argv;
+
   static void load(int argc, char **argv);
+  void cleanup();
 };
 
 #endif // INCLUDES_MYSQL_INSTANCE_MANAGER_OPTIONS_H

server-tools/instance-manager/thread_registry.cc

@@ -197,6 +197,7 @@ void Thread_registry::deliver_shutdown()
     if (info->current_cond)
       pthread_cond_signal(info->current_cond);
   }
+
   pthread_mutex_unlock(&LOCK_thread_registry);
 }
 

server-tools/instance-manager/user_map.cc

@@ -128,9 +128,10 @@ int User_map::load(const char *password_file_name)
 
   if ((file= my_fopen(password_file_name, O_RDONLY | O_BINARY, MYF(0))) == 0)
   {
-    log_error("can't open password file %s: errno=%d, %s", password_file_name,
+    /* Probably the password file wasn't specified. Try to leave without it */
+    log_info("can't open password file %s: errno=%d, %s", password_file_name,
               errno, strerror(errno));
-    return 1;
+    return 0;
   }
 
   while (fgets(line, sizeof(line), file))