Commit aeec4593 authored by unknown's avatar unknown
Browse files

Fix for BUG#17486: IM: race condition on exit. 

The problem was that IM stoped guarded instances on shutdown,
but didn't wait for them to stop.

The fix is to wait for guarded instances to stop before exitting
from the main thread.

The idea is that Instance-monitoring thread should add itself
to Thread_registry so that it will be taken into account on shutdown.
However, Thread_registry should not signal it on shutdown in order to
not interrupt wait()/waitpid().


server-tools/instance-manager/guardian.cc:
  Be more verbose.
server-tools/instance-manager/instance.cc:
  Register mysqld-monitoring thread in Thread_registry.
server-tools/instance-manager/instance.h:
  Add reference to Thread_registry.
server-tools/instance-manager/instance_map.cc:
  Pass Thread_registry reference to Instance.
server-tools/instance-manager/instance_map.h:
  Add reference to Thread_registry.
server-tools/instance-manager/listener.cc:
  Be more verbose.
server-tools/instance-manager/manager.cc:
  Be more verbose.
server-tools/instance-manager/mysql_connection.cc:
  Eliminate type-conversion warnings.
server-tools/instance-manager/thread_registry.cc:
  Be more verbose.
server-tools/instance-manager/thread_registry.h:
  Eliminate copy&paste, make impl-specific constructor private.
parent 66b87280

server-tools/instance-manager/guardian.cc

+7 −1
Original line number Diff line number Diff line
@@ -74,7 +74,7 @@ Guardian_thread::Guardian_thread(Thread_registry &thread_registry_arg, 
                                 uint monitoring_interval_arg) :

  Guardian_thread_args(thread_registry_arg, instance_map_arg,

                       monitoring_interval_arg),

  thread_info(pthread_self()), guarded_instances(0)

  thread_info(pthread_self(), TRUE), guarded_instances(0)

{

  pthread_mutex_init(&LOCK_guardian, 0);

  pthread_cond_init(&COND_guardian, 0);
@@ -250,6 +250,8 @@ void Guardian_thread::run() 
  LIST *node;

  struct timespec timeout;



  log_info("Guardian: started.");



  thread_registry.register_thread(&thread_info);



  my_thread_init();
@@ -277,12 +279,16 @@ void Guardian_thread::run() 
                                     &LOCK_guardian, &timeout);

  }



  log_info("Guardian: stopped.");



  stopped= TRUE;

  pthread_mutex_unlock(&LOCK_guardian);

  /* now, when the Guardian is stopped we can stop the IM */

  thread_registry.unregister_thread(&thread_info);

  thread_registry.request_shutdown();

  my_thread_end();



  log_info("Guardian: finished.");

}

server-tools/instance-manager/instance.cc

+34 −9
Original line number Diff line number Diff line
@@ -34,6 +34,7 @@ 
#include "mysql_manager_error.h"

#include "portability.h"

#include "priv.h"

#include "thread_registry.h"





const LEX_STRING
@@ -44,7 +45,8 @@ static const int INSTANCE_NAME_PREFIX_LEN= Instance::DFLT_INSTANCE_NAME.length; 




static void start_and_monitor_instance(Instance_options *old_instance_options,

                                       Instance_map *instance_map);

                                       Instance_map *instance_map,

                                       Thread_registry *thread_registry);



#ifndef __WIN__

typedef pid_t My_process_info;
@@ -63,7 +65,8 @@ pthread_handler_t proxy(void *arg) 
{

  Instance *instance= (Instance *) arg;

  start_and_monitor_instance(&instance->options,

                             instance->get_map());

                             instance->get_map(),

                             &instance->thread_registry);

  return 0;

}
@@ -99,6 +102,7 @@ static int wait_process(My_process_info *pi) 
    thread, but we don't know this one). Or we could use waitpid(), but

    couldn't use wait(), because it could return in any wait() in the program.

  */



  if (linuxthreads)

    wait(NULL);                               /* LinuxThreads were detected */

  else
@@ -239,11 +243,28 @@ static int start_process(Instance_options *instance_options, 
*/



static void start_and_monitor_instance(Instance_options *old_instance_options,

                                       Instance_map *instance_map)

                                       Instance_map *instance_map,

                                       Thread_registry *thread_registry)

{

  Instance_name instance_name(&old_instance_options->instance_name);

  Instance *current_instance;

  My_process_info process_info;

  Thread_info thread_info(pthread_self(), FALSE);



  log_info("Monitoring thread (instance: '%s'): started.",

           (const char *) instance_name.get_c_str());



  if (!old_instance_options->nonguarded)

  {

    /*

      Register thread in Thread_registry to wait for it to stop on shutdown

      only if instance is nuarded. If instance is guarded, the thread will not

      finish, because nonguarded instances are not stopped on shutdown.

    */



    thread_registry->register_thread(&thread_info);

    my_thread_init();

  }



  /*

    Lock instance map to guarantee that no instances are deleted during
@@ -280,7 +301,14 @@ static void start_and_monitor_instance(Instance_options *old_instance_options, 


  instance_map->unlock();



  return;

  if (!old_instance_options->nonguarded)

  {

    thread_registry->unregister_thread(&thread_info);

    my_thread_end();

  }



  log_info("Monitoring thread (instance: '%s'): finished.",

           (const char *) instance_name.get_c_str());

}
@@ -343,10 +371,6 @@ int Instance::start() 
  {

    remove_pid();



    /*

      No need to monitor this thread in the Thread_registry, as all

      instances are to be stopped during shutdown.

    */

    pthread_t proxy_thd_id;

    pthread_attr_t proxy_thd_attr;

    int rc;
@@ -404,7 +428,8 @@ void Instance::set_crash_flag_n_wake_all() 






Instance::Instance(): crashed(FALSE), configured(FALSE)

Instance::Instance(Thread_registry &thread_registry_arg):

  crashed(FALSE), configured(FALSE), thread_registry(thread_registry_arg)

{

  pthread_mutex_init(&LOCK_instance, 0);

  pthread_cond_init(&COND_instance_stopped, 0);

server-tools/instance-manager/instance.h

+3 −1
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ 
#endif



class Instance_map;

class Thread_registry;





/*
@@ -87,7 +88,7 @@ class Instance 
  static bool is_mysqld_compatible_name(const LEX_STRING *name);



public:

  Instance();

  Instance(Thread_registry &thread_registry_arg);



  ~Instance();

  int init(const LEX_STRING *name_arg);
@@ -120,6 +121,7 @@ class Instance 
public:

  enum { DEFAULT_SHUTDOWN_DELAY= 35 };

  Instance_options options;

  Thread_registry &thread_registry;



private:

  /* This attributes is a flag, specifies if the instance has been crashed. */

server-tools/instance-manager/instance_map.cc

+6 −4
Original line number Diff line number Diff line
@@ -169,7 +169,7 @@ int Instance_map::process_one_option(const LEX_STRING *group, 
  if (!(instance= (Instance *) hash_search(&hash, (byte *) group->str,

                                           group->length)))

  {

    if (!(instance= new Instance()))

    if (!(instance= new Instance(thread_registry)))

      return 1;



    if (instance->init(group) || add_instance(instance))
@@ -213,8 +213,10 @@ int Instance_map::process_one_option(const LEX_STRING *group, 
}





Instance_map::Instance_map(const char *default_mysqld_path_arg):

mysqld_path(default_mysqld_path_arg)

Instance_map::Instance_map(const char *default_mysqld_path_arg,

                           Thread_registry &thread_registry_arg):

  mysqld_path(default_mysqld_path_arg),

  thread_registry(thread_registry_arg)

{

  pthread_mutex_init(&LOCK_instance_map, 0);

}
@@ -333,7 +335,7 @@ int Instance_map::remove_instance(Instance *instance) 
int Instance_map::create_instance(const LEX_STRING *instance_name,

                                  const Named_value_arr *options)

{

  Instance *instance= new Instance();

  Instance *instance= new Instance(thread_registry);



  if (!instance)

  {

server-tools/instance-manager/instance_map.h

+5 −1
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ 
class Guardian_thread;

class Instance;

class Named_value_arr;

class Thread_registry;



extern int load_all_groups(char ***groups, const char *filename);

extern void free_groups(char **groups);
@@ -104,7 +105,8 @@ class Instance_map 
  int create_instance(const LEX_STRING *instance_name,

                      const Named_value_arr *options);



  Instance_map(const char *default_mysqld_path_arg);

  Instance_map(const char *default_mysqld_path_arg,

               Thread_registry &thread_registry_arg);

  ~Instance_map();



  /*
@@ -130,6 +132,8 @@ class Instance_map 
  enum { START_HASH_SIZE = 16 };

  pthread_mutex_t LOCK_instance_map;

  HASH hash;



  Thread_registry &thread_registry;

};



#endif /* INCLUDES_MYSQL_INSTANCE_MANAGER_INSTANCE_MAP_H */