Stop blocking ruby during long-running operations

ext/lxc/lxc.c

@@ -5,6 +5,7 @@
 #include <lxc/attach_options.h>
 #include <stdint.h>
 #include <string.h>
+#include <signal.h>
 
 #define SYMBOL(s) ID2SYM(rb_intern(s))
 
@@ -383,9 +384,23 @@ container_state(VALUE self)
     return rb_str_intern(rb_funcall(rb_state, rb_intern("downcase"), 0));
 }
 
+struct add_device_node_outside_gil_args
+{
+    struct container_data *data;
+    char *src_path;
+    char *dest_path;
+};
+
+static VALUE
+add_device_node_outside_gil(void *args_void)
+{
+    struct add_device_node_outside_gil_args *args = (struct add_device_node_outside_gil_args *)args_void;
+    return INT2NUM( args->data->container->add_device_node(args->data->container, args->src_path, args->dest_path) );
+}
+
 /*
  * call-seq:
- *   container.add_device_node(src_path, dst_path = src_path)
+ *   container.add_device_node(src_path, dest_path = src_path)
  *
  * Adds a device node to the container.
  */
@@ -393,17 +408,16 @@ static VALUE
 container_add_device_node(int argc, VALUE *argv, VALUE self)
 {
     int ret;
-    char *src_path, *dst_path;
+    VALUE rb_src_path, rb_dest_path;
-    struct container_data *data;
+    struct add_device_node_outside_gil_args args;
-    VALUE rb_src_path, rb_dst_path;
 
-    rb_scan_args(argc, argv, "11", &rb_src_path, &rb_dst_path);
+    rb_scan_args(argc, argv, "11", &rb_src_path, &rb_dest_path);
-    src_path = NIL_P(rb_src_path) ? NULL : StringValuePtr(rb_src_path);
+    args.src_path = StringValuePtr(rb_src_path);
-    dst_path = NIL_P(rb_dst_path) ? NULL : StringValuePtr(rb_dst_path);
+    args.dest_path = NIL_P(rb_dest_path) ? NULL : StringValuePtr(rb_dest_path);
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
 
-    ret = data->container->add_device_node(data->container, src_path, dst_path);
+    ret = NUM2INT(rb_thread_blocking_region(add_device_node_outside_gil, &args, NULL, NULL));
     if (!ret)
         rb_raise(Error, "unable to add device node");
 
@@ -593,6 +607,15 @@ err:
     return NULL;
 }
 
+static VALUE lxc_wait_for_pid_status_without_gil(void* pid)
+{
+  return INT2NUM(lxc_wait_for_pid_status(*(pid_t*)pid));
+}
+static void kill_pid_without_gil(void* pid)
+{
+  kill(*(pid_t*)pid, SIGKILL);
+}
+
 /*
  * call-seq:
  *   container.attach(opts = {}, &block)
@@ -650,7 +673,13 @@ container_attach(int argc, VALUE *argv, VALUE self)
         goto out;
 
     if (wait) {
-        ret = lxc_wait_for_pid_status(pid);
+        ret = NUM2INT(
+          rb_thread_blocking_region(
+            lxc_wait_for_pid_status_without_gil,
+            &pid,
+            kill_pid_without_gil,
+            &pid
+          ));
         /* handle case where attach fails */
         if (WIFEXITED(ret) && WEXITSTATUS(ret) == 255)
             ret = -1;
@@ -702,6 +731,30 @@ container_clear_config_item(VALUE self, VALUE rb_key)
     return self;
 }
 
+struct clone_outside_gil_args
+{
+    struct container_data *data;
+    struct lxc_container *new_container;
+    char *name;
+    char *config_path;
+    int flags;
+    char *bdev_type;
+    char *bdev_data;
+    uint64_t new_size;
+    char **hook_args;
+};
+
+static VALUE
+clone_outside_gil(void *args_void)
+{
+    struct clone_outside_gil_args *args = (struct clone_outside_gil_args *)args_void;
+    struct lxc_container *container = args->data->container;
+    args->new_container = container->clone(container, args->name,
+        args->config_path, args->flags, args->bdev_type, args->bdev_data,
+        args->new_size, args->hook_args);
+    return Qnil;
+}
+
 /*
  * call-seq:
  *   container.clone(clone_name, opts = {})
@@ -719,27 +772,22 @@ container_clear_config_item(VALUE self, VALUE rb_key)
 static VALUE
 container_clone(int argc, VALUE *argv, VALUE self)
 {
-    int flags;
-    uint64_t new_size;
-    char *name, *config_path, *bdev_type, *bdev_data;
-    char **hook_args;
-    struct lxc_container *container, *new_container;
-    struct container_data *data;
     VALUE rb_name, rb_opts;
     VALUE rb_flags, rb_config_path, rb_bdev_type, rb_bdev_data;
     VALUE rb_new_size, rb_hook_args;
     VALUE rb_args[2];
+    struct clone_outside_gil_args args;
 
     rb_scan_args(argc, argv, "11", &rb_name, &rb_opts);
 
-    name = StringValuePtr(rb_name);
+    args.name = StringValuePtr(rb_name);
 
-    config_path = NULL;
+    args.config_path = NULL;
-    flags       = 0;
+    args.flags       = 0;
-    bdev_type   = NULL;
+    args.bdev_type   = NULL;
-    bdev_data   = NULL;
+    args.bdev_data   = NULL;
-    new_size    = 0;
+    args.new_size    = 0;
-    hook_args   = NULL;
+    args.hook_args   = NULL;
 
     rb_config_path = Qnil;
 
@@ -747,49 +795,65 @@ container_clone(int argc, VALUE *argv, VALUE self)
         Check_Type(rb_opts, T_HASH);
         rb_config_path = rb_hash_aref(rb_opts, SYMBOL("config_path"));
         if (!NIL_P(rb_config_path))
-            config_path = StringValuePtr(rb_config_path);
+            args.config_path = StringValuePtr(rb_config_path);
 
         rb_flags = rb_hash_aref(rb_opts, SYMBOL("flags"));
         if (!NIL_P(rb_flags))
-            flags = NUM2INT(rb_flags);
+            args.flags = NUM2INT(rb_flags);
 
         rb_bdev_type = rb_hash_aref(rb_opts, SYMBOL("bdev_type"));
         if (!NIL_P(rb_bdev_type))
-            bdev_type = StringValuePtr(rb_bdev_type);
+            args.bdev_type = StringValuePtr(rb_bdev_type);
 
         rb_bdev_data = rb_hash_aref(rb_opts, SYMBOL("bdev_data"));
         if (!NIL_P(rb_bdev_data))
-            bdev_data = StringValuePtr(rb_bdev_data);
+            args.bdev_data = StringValuePtr(rb_bdev_data);
 
         rb_new_size = rb_hash_aref(rb_opts, SYMBOL("new_size"));
         if (!NIL_P(rb_bdev_data))
-            new_size = NUM2ULL(rb_new_size);
+            args.new_size = NUM2ULL(rb_new_size);
 
         rb_hook_args = rb_hash_aref(rb_opts, SYMBOL("hook_args"));
         if (!NIL_P(rb_hook_args))
-            hook_args = ruby_to_c_string_array(rb_hook_args);
+            args.hook_args = ruby_to_c_string_array(rb_hook_args);
     }
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
-    container = data->container;
 
-    new_container = container->clone(container, name, config_path,
+    rb_thread_blocking_region(clone_outside_gil, &args, NULL, NULL);
-                                     flags, bdev_type, bdev_data, new_size,
-                                     hook_args);
 
-    if (hook_args)
+    if (args.hook_args)
-        free_c_string_array(hook_args);
+        free_c_string_array(args.hook_args);
 
-    if (new_container == NULL)
+    if (args.new_container == NULL)
         rb_raise(Error, "unable to clone container");
 
-    lxc_container_put(new_container);
+    lxc_container_put(args.new_container);
 
     rb_args[0] = rb_name;
     rb_args[1] = rb_config_path;
     return rb_class_new_instance(2, rb_args, Container);
 }
 
+struct console_outside_gil_args
+{
+    struct container_data *data;
+    int tty_num;
+    int stdin_fd;
+    int stdout_fd;
+    int stderr_fd;
+    int escape;
+};
+
+static VALUE
+console_outside_gil(void *args_void)
+{
+    struct console_outside_gil_args *args = (struct console_outside_gil_args *)args_void;
+    return INT2NUM( args->data->container->console(args->data->container, args->tty_num,
+                                                   args->stdin_fd, args->stdout_fd,
+                                                   args->stderr_fd, args->escape) );
+}
+
 /*
  * call-seq:
  *   container.console(opts = {})
@@ -807,29 +871,44 @@ static VALUE
 container_console(int argc, VALUE *argv, VALUE self)
 {
     int ret;
-    int tty_num = -1, stdin_fd = 0, stdout_fd = 1, stderr_fd = 2, escape = 1;
+    struct console_outside_gil_args args;
-    struct container_data *data;
-    struct lxc_container *container;
     VALUE rb_opts;
+    VALUE rb_opt;
+
+    args.tty_num = -1;
+    args.stdin_fd = 0;
+    args.stdout_fd = 1;
+    args.stdout_fd = 2;
+    args.escape = 1;
 
     rb_scan_args(argc, argv, "01", &rb_opts);
     switch (TYPE(rb_opts)) {
     case T_HASH:
-        tty_num   = NUM2INT(rb_hash_aref(rb_opts, SYMBOL("tty_num")));
+        rb_opt = rb_hash_aref(rb_opts, SYMBOL("tty_num"));
-        stdin_fd  = NUM2INT(rb_hash_aref(rb_opts, SYMBOL("stdin_fd")));
+        if (!NIL_P(rb_opt))
-        stdout_fd = NUM2INT(rb_hash_aref(rb_opts, SYMBOL("stdout_fd")));
+          args.tty_num = NUM2INT(rb_opt);
-        stderr_fd = NUM2INT(rb_hash_aref(rb_opts, SYMBOL("stderr_fd")));
+        rb_opt = rb_hash_aref(rb_opts, SYMBOL("stdin_fd"));
-        escape    = NUM2INT(rb_hash_aref(rb_opts, SYMBOL("escape")));
+        if (!NIL_P(rb_opt))
+          args.stdin_fd = NUM2INT(rb_opt);
+        rb_opt = rb_hash_aref(rb_opts, SYMBOL("stdout_fd"));
+        if (!NIL_P(rb_opt))
+          args.stdout_fd = NUM2INT(rb_opt);
+        rb_opt = rb_hash_aref(rb_opts, SYMBOL("stderr_fd"));
+        if (!NIL_P(rb_opt))
+          args.stderr_fd = NUM2INT(rb_opt);
+        rb_opt = rb_hash_aref(rb_opts, SYMBOL("escape"));
+        if (!NIL_P(rb_opt))
+          args.escape = NUM2INT(rb_opt);
+        break;
+    case T_NIL:
         break;
     default:
         rb_raise(rb_eArgError, "options must be a hash");
     }
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
-    container = data->container;
 
-    ret = container->console(container, tty_num, stdin_fd, stdout_fd, stderr_fd,
+    ret = NUM2INT(rb_thread_blocking_region(console_outside_gil, &args, NULL, NULL));
-                             escape);
     if (ret != 0)
         rb_raise(Error, "unable to access container console");
 
@@ -863,6 +942,23 @@ container_console_fd(int argc, VALUE *argv, VALUE self)
     return rb_class_new_instance(1, rb_io_args, rb_cIO);
 }
 
+/* Used to run container->create outside of GIL */
+struct container_create_outside_gil_args {
+    struct container_data *data;
+    char *template;
+    char *bdevtype;
+    int flags;
+    char **args;
+};
+
+static VALUE
+container_create_outside_gil(void *args_void)
+{
+    struct container_create_outside_gil_args *args = (struct container_create_outside_gil_args *)args_void;
+    int ret = args->data->container->create(args->data->container, args->template, args->bdevtype, NULL, args->flags, args->args);
+    return INT2NUM(ret);
+}
+
 /*
  * call-seq:
  *   container.create(template, bdevtype = nil, flags = 0, args = [])
@@ -876,28 +972,26 @@ container_console_fd(int argc, VALUE *argv, VALUE self)
 static VALUE
 container_create(int argc, VALUE *argv, VALUE self)
 {
-    int ret, flags;
+    int ret;
-    char *template;
-    char *bdevtype;
-    char **args = { NULL };
-    struct container_data *data;
-    struct lxc_container *container;
     VALUE rb_template, rb_bdevtype, rb_flags, rb_args;
+    struct container_create_outside_gil_args args;
+    char ** default_args = { NULL };
 
+    args.args = default_args;
     rb_scan_args(argc, argv, "13", &rb_template, &rb_bdevtype, &rb_flags, &rb_args);
 
-    template = StringValuePtr(rb_template);
+    args.template = StringValuePtr(rb_template);
-    bdevtype = NIL_P(rb_bdevtype) ? NULL : StringValuePtr(rb_bdevtype);
+    args.bdevtype = NIL_P(rb_bdevtype) ? NULL : StringValuePtr(rb_bdevtype);
-    flags = NIL_P(rb_flags) ? 0 : NUM2INT(rb_flags);
+    args.flags = NIL_P(rb_flags) ? 0 : NUM2INT(rb_flags);
     if (!NIL_P(rb_args))
-        args = ruby_to_c_string_array(rb_args);
+        args.args = ruby_to_c_string_array(rb_args);
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
-    container = data->container;
+
-    ret = container->create(container, template, bdevtype, NULL, flags, args);
+    ret = NUM2INT(rb_thread_blocking_region(container_create_outside_gil, &args, NULL, NULL));
 
     if (!NIL_P(rb_args))
-        free_c_string_array(args);
+        free_c_string_array(args.args);
 
     if (!ret)
         rb_raise(Error, "unable to create container");
@@ -905,6 +999,14 @@ container_create(int argc, VALUE *argv, VALUE self)
     return self;
 }
 
+
+static VALUE
+destroy_outside_gil(void* data_void)
+{
+  struct container_data *data = (struct container_data *)data_void;
+  return INT2NUM( data->container->destroy(data->container) );
+}
+
 /*
  * call-seq:
  *   container.destroy
@@ -919,12 +1021,19 @@ container_destroy(VALUE self)
 
     Data_Get_Struct(self, struct container_data, data);
 
-    ret = data->container->destroy(data->container);
+    ret = NUM2INT(rb_thread_blocking_region(destroy_outside_gil, data, NULL, NULL));
     if (!ret)
         rb_raise(Error, "unable to destroy container");
     return self;
 }
 
+static VALUE
+freeze_outside_gil(void* data_void)
+{
+  struct container_data *data = (struct container_data *)data_void;
+  return INT2NUM( data->container->freeze(data->container) );
+}
+
 /*
  * call-seq:
  *   container.freeze
@@ -939,7 +1048,7 @@ container_freeze(VALUE self)
 
     Data_Get_Struct(self, struct container_data, data);
 
-    ret = data->container->freeze(data->container);
+    ret = NUM2INT(rb_thread_blocking_region(freeze_outside_gil, data, NULL, NULL));
     if (!ret)
         rb_raise(Error, "unable to freeze container");
 
@@ -1146,6 +1255,19 @@ container_ips(int argc, VALUE *argv, VALUE self)
     return rb_ips;
 }
 
+struct load_config_outside_gil_args
+{
+    struct container_data *data;
+    char *path;
+};
+
+static VALUE
+load_config_outside_gil(void *args_void)
+{
+    struct load_config_outside_gil_args *args = (struct load_config_outside_gil_args *)args_void;
+    return INT2NUM( args->data->container->load_config(args->data->container, args->path) );
+}
+
 /*
  * call-seq:
  *   container.load_config(config_path = nil)
@@ -1156,22 +1278,28 @@ static VALUE
 container_load_config(int argc, VALUE *argv, VALUE self)
 {
     int ret;
-    char *path;
-    struct container_data *data;
     VALUE rb_path;
+    struct load_config_outside_gil_args args;
 
     rb_scan_args(argc, argv, "01", &rb_path);
-    path = NIL_P(rb_path) ? NULL : StringValuePtr(rb_path);
+    args.path = NIL_P(rb_path) ? NULL : StringValuePtr(rb_path);
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
 
-    ret = data->container->load_config(data->container, path);
+    ret = NUM2INT(rb_thread_blocking_region(load_config_outside_gil, &args, NULL, NULL));
     if (!ret)
         rb_raise(Error, "unable to load configuration file");
 
     return self;
 }
 
+static VALUE
+reboot_outside_gil(void* data_void)
+{
+    struct container_data *data = (struct container_data *)data_void;
+    return INT2NUM( data->container->reboot(data->container) );
+}
+
 /*
  * call-seq:
  *   container.reboot
@@ -1186,16 +1314,30 @@ container_reboot(VALUE self)
 
     Data_Get_Struct(self, struct container_data, data);
 
-    ret = data->container->reboot(data->container);
+    ret = NUM2INT(rb_thread_blocking_region(reboot_outside_gil, data, NULL, NULL));
     if (!ret)
         rb_raise(Error, "unable to reboot container");
 
     return self;
 }
 
+struct remove_device_node_outside_gil_args
+{
+    struct container_data *data;
+    char *src_path;
+    char *dest_path;
+};
+
+static VALUE
+remove_device_node_outside_gil(void *args_void)
+{
+    struct remove_device_node_outside_gil_args *args = (struct remove_device_node_outside_gil_args *)args_void;
+    return INT2NUM( args->data->container->remove_device_node(args->data->container, args->src_path, args->dest_path) );
+}
+
 /*
  * call-seq:
- *   container.remove_device_node(src_path, dst_path = src_path)
+ *   container.remove_device_node(src_path, dest_path = src_path)
  *
  * Removes a device node from the container.
  */
@@ -1203,25 +1345,35 @@ static VALUE
 container_remove_device_node(int argc, VALUE *argv, VALUE self)
 {
     int ret;
-    char *src_path, *dst_path;
+    VALUE rb_src_path, rb_dest_path;
-    struct lxc_container *container;
+    struct remove_device_node_outside_gil_args args;
-    struct container_data *data;
-    VALUE rb_src_path, rb_dst_path;
 
-    rb_scan_args(argc, argv, "11", &rb_src_path, &rb_dst_path);
+    rb_scan_args(argc, argv, "11", &rb_src_path, &rb_dest_path);
-    src_path = StringValuePtr(rb_src_path);
+    args.src_path = StringValuePtr(rb_src_path);
-    dst_path = NIL_P(rb_dst_path) ? NULL : StringValuePtr(rb_dst_path);
+    args.dest_path = NIL_P(rb_dest_path) ? NULL : StringValuePtr(rb_dest_path);
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
-    container = data->container;
 
-    ret = container->remove_device_node(container, src_path, dst_path);
+    ret = NUM2INT(rb_thread_blocking_region(remove_device_node_outside_gil, &args, NULL, NULL));
     if (!ret)
         rb_raise(Error, "unable to remove device node");
 
     return self;
 }
 
+struct rename_outside_gil_args
+{
+    struct container_data *data;
+    char *name;
+};
+
+static VALUE
+rename_outside_gil(void* args_void)
+{
+    struct rename_outside_gil_args *args = (struct rename_outside_gil_args *)args_void;
+    return INT2NUM( args->data->container->rename(args->data->container, args->name) );
+}
+
 /*
  * call-seq:
  *   container.rename(new_name)
@@ -1233,14 +1385,14 @@ static VALUE
 container_rename(VALUE self, VALUE rb_name)
 {
     int ret;
-    char *name;
-    struct container_data *data;
     VALUE rb_args[2];
+    struct rename_outside_gil_args args;
 
-    name = StringValuePtr(rb_name);
+    Data_Get_Struct(self, struct container_data, args.data);
-    Data_Get_Struct(self, struct container_data, data);
 
-    ret = data->container->rename(data->container, name);
+    args.name = StringValuePtr(rb_name);
+
+    ret = NUM2INT(rb_thread_blocking_region(rename_outside_gil, &args, NULL, NULL));
     if (!ret)
         rb_raise(Error, "unable to rename container");
 
@@ -1278,20 +1430,32 @@ container_running_config_item(VALUE self, VALUE rb_key)
     return rb_value;
 }
 
+struct save_config_outside_gil_args
+{
+    struct container_data *data;
+    char *path;
+};
+
+static VALUE
+save_config_outside_gil(void *args_void)
+{
+  struct save_config_outside_gil_args *args = (struct save_config_outside_gil_args *)args_void;
+  return INT2NUM( args->data->container->save_config(args->data->container, args->path) );
+}
+
 static VALUE
 container_save_config(int argc, VALUE *argv, VALUE self)
 {
     int ret;
-    char *path;
-    struct container_data *data;
     VALUE rb_path;
+    struct save_config_outside_gil_args args;
 
     rb_scan_args(argc, argv, "01", &rb_path);
-    path = NIL_P(rb_path) ? NULL : StringValuePtr(rb_path);
+    args.path = NIL_P(rb_path) ? NULL : StringValuePtr(rb_path);
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
 
-    ret = data->container->save_config(data->container, path);
+    ret = NUM2INT(rb_thread_blocking_region(save_config_outside_gil, &args, NULL, NULL));
     if (!ret)
         rb_raise(Error, "unable to save configuration file");
 
@@ -1391,6 +1555,19 @@ container_set_config_path(VALUE self, VALUE rb_path)
     return self;
 }
 
+struct shutdown_outside_gil_args
+{
+    struct container_data *data;
+    int timeout;
+};
+
+static VALUE
+shutdown_outside_gil(void* args_void)
+{
+    struct shutdown_outside_gil_args *args = (struct shutdown_outside_gil_args *)args_void;
+    return INT2NUM( args->data->container->shutdown(args->data->container, args->timeout) );
+}
+
 /*
  * call-seq:
  *   container.shutdown(timeout = -1)
@@ -1402,22 +1579,36 @@ container_set_config_path(VALUE self, VALUE rb_path)
 static VALUE
 container_shutdown(int argc, VALUE *argv, VALUE self)
 {
-    int ret, timeout;
+    int ret;
-    struct container_data *data;
     VALUE rb_timeout;
+    struct shutdown_outside_gil_args args;
 
     rb_scan_args(argc, argv, "01", &rb_timeout);
-    timeout = NIL_P(rb_timeout) ? -1 : NUM2INT(rb_timeout);
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
 
-    ret = data->container->shutdown(data->container, timeout);
+    args.timeout = NIL_P(rb_timeout) ? -1 : NUM2INT(rb_timeout);
+
+    ret = NUM2INT(rb_thread_blocking_region(shutdown_outside_gil, &args, NULL, NULL));
     if (!ret)
         rb_raise(Error, "unable to shutdown container");
 
     return self;
 }
 
+struct snapshot_outside_gil_args
+{
+    struct container_data *data;
+    char *path;
+};
+
+static VALUE
+snapshot_outside_gil(void* args_void)
+{
+    struct snapshot_outside_gil_args *args = (struct snapshot_outside_gil_args *)args_void;
+    return INT2NUM( args->data->container->snapshot(args->data->container, args->path) );
+}
+
 /*
  * call-seq:
  *   container.snapshot(path = nil)
@@ -1428,17 +1619,16 @@ static VALUE
 container_snapshot(int argc, VALUE *argv, VALUE self)
 {
     int ret;
-    char *path;
     char new_name[20];
-    struct container_data *data;
     VALUE rb_path;
+    struct snapshot_outside_gil_args args;
 
     rb_scan_args(argc, argv, "01", &rb_path);
-    path = NIL_P(rb_path) ? NULL : StringValuePtr(rb_path);
+    args.path = NIL_P(rb_path) ? NULL : StringValuePtr(rb_path);
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
 
-    ret = data->container->snapshot(data->container, path);
+    ret = NUM2INT(rb_thread_blocking_region(snapshot_outside_gil, &args, NULL, NULL));
     if (ret < 0)
         rb_raise(Error, "unable to snapshot container");
 
@@ -1449,6 +1639,19 @@ container_snapshot(int argc, VALUE *argv, VALUE self)
     return rb_str_new2(new_name);
 }
 
+struct snapshot_destroy_outside_gil_args
+{
+    struct container_data *data;
+    char *name;
+};
+
+static VALUE
+snapshot_destroy_outside_gil(void* args_void)
+{
+    struct snapshot_destroy_outside_gil_args *args = (struct snapshot_destroy_outside_gil_args *)args_void;
+    return INT2NUM( args->data->container->snapshot_destroy(args->data->container, args->name) );
+}
+
 /*
  * call-seq:
  *   container.snapshot_destroy(name)
@@ -1459,20 +1662,32 @@ static VALUE
 container_snapshot_destroy(VALUE self, VALUE rb_name)
 {
     int ret;
-    char *name;
+    struct snapshot_destroy_outside_gil_args args;
-    struct container_data *data;
 
-    name = StringValuePtr(rb_name);
+    Data_Get_Struct(self, struct container_data, args.data);
 
-    Data_Get_Struct(self, struct container_data, data);
+    args.name = StringValuePtr(rb_name);
 
-    ret = data->container->snapshot_destroy(data->container, name);
+    ret = NUM2INT(rb_thread_blocking_region(snapshot_destroy_outside_gil, &args, NULL, NULL));
     if (!ret)
         rb_raise(Error, "unable to destroy snapshot");
 
     return self;
 }
 
+struct snapshot_list_outside_gil_args
+{
+    struct container_data *data;
+    struct lxc_snapshot *snapshots;
+};
+
+static VALUE
+snapshot_list_outside_gil(void* args_void)
+{
+    struct snapshot_list_outside_gil_args *args = (struct snapshot_list_outside_gil_args *)args_void;
+    return INT2NUM( args->data->container->snapshot_list(args->data->container, &args->snapshots) );
+}
+
 /*
  * call-seq:
  *   container.snapshot_list
@@ -1483,30 +1698,43 @@ static VALUE
 container_snapshot_list(VALUE self)
 {
     int i, num_snapshots;
-    struct lxc_snapshot *snapshots;
-    struct container_data *data;
     VALUE rb_snapshots;
+    struct snapshot_list_outside_gil_args args;
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
 
-    num_snapshots = data->container->snapshot_list(data->container, &snapshots);
+    num_snapshots = NUM2INT(rb_thread_blocking_region(snapshot_list_outside_gil, &args, NULL, NULL));
     if (num_snapshots < 0)
         rb_raise(Error, "unable to list snapshots");
 
     rb_snapshots = rb_ary_new2(num_snapshots);
     for (i = 0; i < num_snapshots; i++) {
         VALUE attrs = rb_ary_new2(4);
-        rb_ary_store(attrs, 0, rb_str_new2(snapshots[i].name));
+        rb_ary_store(attrs, 0, rb_str_new2(args.snapshots[i].name));
-        rb_ary_store(attrs, 1, rb_str_new2(snapshots[i].comment_pathname));
+        rb_ary_store(attrs, 1, rb_str_new2(args.snapshots[i].comment_pathname));
-        rb_ary_store(attrs, 2, rb_str_new2(snapshots[i].timestamp));
+        rb_ary_store(attrs, 2, rb_str_new2(args.snapshots[i].timestamp));
-        rb_ary_store(attrs, 3, rb_str_new2(snapshots[i].lxcpath));
+        rb_ary_store(attrs, 3, rb_str_new2(args.snapshots[i].lxcpath));
-        snapshots[i].free(&snapshots[i]);
+        args.snapshots[i].free(&args.snapshots[i]);
         rb_ary_store(rb_snapshots, i, attrs);
     }
 
     return rb_snapshots;
 }
 
+struct snapshot_restore_outside_gil_args
+{
+    struct container_data *data;
+    char *name;
+    char *new_name;
+};
+
+static VALUE
+snapshot_restore_outside_gil(void* args_void)
+{
+    struct snapshot_restore_outside_gil_args *args = (struct snapshot_restore_outside_gil_args *)args_void;
+    return INT2NUM( args->data->container->snapshot_restore(args->data->container, args->name, args->new_name) );
+}
+
 /*
  * call-seq:
  *   container.snapshot_restore(name, new_name = nil)
@@ -1517,23 +1745,42 @@ static VALUE
 container_snapshot_restore(int argc, VALUE *argv, VALUE self)
 {
     int ret;
-    char *name, *new_name;
+    struct snapshot_restore_outside_gil_args args;
-    struct container_data *data;
     VALUE rb_name, rb_new_name;
 
     rb_scan_args(argc, argv, "11", &rb_name, &rb_new_name);
-    name = StringValuePtr(rb_name);
+    args.name = StringValuePtr(rb_name);
-    new_name = NIL_P(rb_new_name) ? NULL : StringValuePtr(rb_new_name);
+    args.new_name = NIL_P(rb_new_name) ? NULL : StringValuePtr(rb_new_name);
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
 
-    ret = data->container->snapshot_restore(data->container, name, new_name);
+    ret = NUM2INT(rb_thread_blocking_region(snapshot_restore_outside_gil, &args, NULL, NULL));
     if (!ret)
         rb_raise(Error, "unable to restore snapshot");
 
     return self;
 }
 
+
+struct start_outside_gil_args
+{
+    struct container_data *data;
+    int use_init;
+    int daemonize;
+    int close_fds;
+    char **args;
+};
+
+static VALUE
+start_outside_gil(void* args_void)
+{
+    struct start_outside_gil_args *args = (struct start_outside_gil_args *)args_void;
+    struct lxc_container *container = args->data->container;
+    container->want_close_all_fds(container, args->close_fds);
+    container->want_daemonize(container, args->daemonize);
+    return INT2NUM( container->start(container, args->use_init, args->args) );
+}
+
 /*
  * call-seq:
  *   container.start(opts = {})
@@ -1548,41 +1795,38 @@ container_snapshot_restore(int argc, VALUE *argv, VALUE self)
 static VALUE
 container_start(int argc, VALUE *argv, VALUE self)
 {
-    int ret, use_init, daemonize, close_fds;
+    int ret;
-    char **args;
-    struct container_data *data;
     VALUE rb_use_init, rb_daemonize, rb_close_fds, rb_args, rb_opts;
+    struct start_outside_gil_args args;
 
-    use_init = 0;
+    args.use_init = 0;
-    daemonize = 1;
+    args.daemonize = 1;
-    close_fds = 0;
+    args.close_fds = 0;
-    args = NULL;
+    args.args = NULL;
     rb_args = Qnil;
 
     rb_scan_args(argc, argv, "01", &rb_opts);
     if (!NIL_P(rb_opts)) {
         Check_Type(rb_opts, T_HASH);
         rb_use_init = rb_hash_aref(rb_opts, SYMBOL("use_init"));
-        use_init = (rb_use_init != Qnil) && (rb_use_init != Qfalse);
+        args.use_init = (rb_use_init != Qnil) && (rb_use_init != Qfalse);
 
         rb_daemonize = rb_hash_aref(rb_opts, SYMBOL("daemonize"));
-        daemonize = (rb_daemonize != Qnil) && (rb_daemonize != Qfalse);
+        args.daemonize = (rb_daemonize != Qnil) && (rb_daemonize != Qfalse);
 
         rb_close_fds = rb_hash_aref(rb_opts, SYMBOL("close_fds"));
-        close_fds = (rb_close_fds != Qnil) && (rb_close_fds != Qfalse);
+        args.close_fds = (rb_close_fds != Qnil) && (rb_close_fds != Qfalse);
 
         rb_args = rb_hash_aref(rb_opts, SYMBOL("args"));
-        args = NIL_P(rb_args) ? NULL : ruby_to_c_string_array(rb_args);
+        args.args = NIL_P(rb_args) ? NULL : ruby_to_c_string_array(rb_args);
     }
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
 
-    data->container->want_close_all_fds(data->container, close_fds);
+    ret = NUM2INT(rb_thread_blocking_region(start_outside_gil, &args, NULL, NULL));
-    data->container->want_daemonize(data->container, daemonize);
-    ret = data->container->start(data->container, use_init, args);
 
     if (!NIL_P(rb_args))
-        free_c_string_array(args);
+        free_c_string_array(args.args);
 
     if (!ret)
         rb_raise(Error, "unable to start container");
@@ -1590,6 +1834,13 @@ container_start(int argc, VALUE *argv, VALUE self)
     return self;
 }
 
+static VALUE
+stop_outside_gil(void* data_void)
+{
+    struct container_data *data = (struct container_data *)data_void;
+    return INT2NUM( data->container->stop(data->container) );
+}
+
 /*
  * call-seq:
  *   container.stop
@@ -1604,13 +1855,20 @@ container_stop(VALUE self)
 
     Data_Get_Struct(self, struct container_data, data);
 
-    ret = data->container->stop(data->container);
+    ret = NUM2INT(rb_thread_blocking_region(stop_outside_gil, data, NULL, NULL));
     if (!ret)
         rb_raise(Error, "unable to stop container");
 
     return self;
 }
 
+static VALUE
+unfreeze_outside_gil(void* data_void)
+{
+  struct container_data *data = (struct container_data *)data_void;
+  return INT2NUM( data->container->unfreeze(data->container) );
+}
+
 /*
  * call-seq:
  *   container.unfreeze
@@ -1625,13 +1883,27 @@ container_unfreeze(VALUE self)
 
     Data_Get_Struct(self, struct container_data, data);
 
-    ret = data->container->unfreeze(data->container);
+    ret = NUM2INT(rb_thread_blocking_region(unfreeze_outside_gil, data, NULL, NULL));
     if (!ret)
-        rb_raise(Error, "unable to unfreeze container");
+        rb_raise(Error, "unable to unfreeze container: #{lxc_strerror(ret)}");
 
     return self;
 }
 
+struct wait_outside_gil_args
+{
+    struct container_data *data;
+    int timeout;
+    char *state;
+};
+
+static VALUE
+wait_outside_gil(void* args_void)
+{
+    struct wait_outside_gil_args *args = (struct wait_outside_gil_args *)args_void;
+    return INT2NUM( args->data->container->wait(args->data->container, args->state, args->timeout) );
+}
+
 /*
  * call-seq:
  *   container.wait(state, timeout = -1)
@@ -1642,22 +1914,21 @@ container_unfreeze(VALUE self)
 static VALUE
 container_wait(int argc, VALUE *argv, VALUE self)
 {
-    int ret, timeout;
+    int ret;
-    char *state;
-    struct container_data *data;
     VALUE rb_state_str, rb_state, rb_timeout;
+    struct wait_outside_gil_args args;
 
     rb_scan_args(argc, argv, "11", &rb_state, &rb_timeout);
 
     rb_state_str = rb_funcall(rb_state, rb_intern("to_s"), 0);
     rb_state_str = rb_funcall(rb_state_str, rb_intern("upcase"), 0);
-    state = StringValuePtr(rb_state_str);
+    args.state = StringValuePtr(rb_state_str);
 
-    timeout = NIL_P(rb_timeout) ? -1 : NUM2INT(rb_timeout);
+    args.timeout = NIL_P(rb_timeout) ? -1 : NUM2INT(rb_timeout);
 
-    Data_Get_Struct(self, struct container_data, data);
+    Data_Get_Struct(self, struct container_data, args.data);
 
-    ret = data->container->wait(data->container, state, timeout);
+    ret = NUM2INT( rb_thread_blocking_region(wait_outside_gil, &args, NULL, NULL) );
     if (!ret)
         rb_raise(Error, "error waiting for container");
 
@@ -1713,7 +1984,7 @@ Init_lxc(void)
     rb_define_method(Container, "load_config", container_load_config, -1);
     rb_define_method(Container, "reboot", container_reboot, 0);
     rb_define_method(Container, "remove_device_node",
-                     container_remove_device_node, 0);
+                     container_remove_device_node, -1);
     rb_define_method(Container, "rename", container_rename, 1);
     rb_define_method(Container, "running_config_item",
                      container_running_config_item, 1);

test/test_helpers.rb

@@ -0,0 +1,27 @@
+require 'lxc'
+require 'test/unit/assertions'
+
+module TestHelpers
+  def assert_long_running_function_does_not_block_ruby(&block)
+    r, w = IO.pipe
+    begin
+      # Write something after a very short period, but only if Ruby isn't blocked!
+      t = Thread.new do
+        sleep(0.001)
+        # Sleep twice so that if the function is blocking and we wake up just
+        # after block.call(c) somehow, we will go back to sleep briefly and
+        # allow B to be written
+        sleep(0.001)
+        w.write('A')
+      end
+      # Call the function and see if Ruby gets blocked
+      block.call
+      w.write('B')
+      chars = r.read(2)
+      assert(chars == 'AB', "Expected thread to write before block finished, but it did not. Expected 'AB', got '#{chars}'")
+    ensure
+      r.close
+      w.close
+    end
+  end
+end

test/test_lxc_create_asynchronous.rb

@@ -0,0 +1,25 @@
+$:.unshift File.expand_path(File.join(File.dirname(__FILE__), 'lib'))
+
+require 'test/unit'
+require 'lxc'
+require 'test_helpers'
+
+class TestLXCCreateAsynchronous < Test::Unit::TestCase
+  include TestHelpers
+
+  def setup
+    if Process::Sys::geteuid != 0
+      raise 'This test must be ran as root'
+    end
+    @name = 'test_async_create'
+    container = LXC::Container.new(@name)
+    container.destroy if container.defined?
+  end
+
+  def test_create_allows_ruby_to_continue
+    c = LXC::Container.new(@name)
+    assert_long_running_function_does_not_block_ruby do
+      c.create('ubuntu')
+    end
+  end
+end

test/test_lxc_created.rb

@@ -6,11 +6,15 @@ require 'lxc'
 class TestLXCCreated < Test::Unit::TestCase
   def setup
     if Process::Sys::geteuid != 0
-      raise 'This test must be ran as root'
+      raise 'This test must be run as root'
     end
     @name = 'test'
     @container = LXC::Container.new(@name)
     @container.create('ubuntu') unless @container.defined?
+    # Make sure the renamed_test container does not exist, for the rename test
+    @new_name = "renamed_#{@name}"
+    new_container = LXC::Container.new(@new_name)
+    new_container.destroy if new_container.defined?
   end
 
   def test_container_defined
@@ -41,9 +45,8 @@ class TestLXCCreated < Test::Unit::TestCase
   end
 
   def test_container_rename
-    new_name = "renamed_#{@name}"
+    renamed = @container.rename(@new_name)
-    renamed = @container.rename(new_name)
+    assert_equal(@new_name, renamed.name)
-    assert_equal(new_name, renamed.name)
     rerenamed = renamed.rename(@name)
     assert_equal(@name, rerenamed.name)
   end

test/test_lxc_long_running.rb

@@ -0,0 +1,228 @@
+$:.unshift File.expand_path(File.join(File.dirname(__FILE__), 'lib'))
+
+require 'test/unit'
+require 'lxc'
+require 'test_helpers'
+
+class TestLXCLongRunning < Test::Unit::TestCase
+  include TestHelpers
+
+  def setup
+    if Process::Sys::geteuid != 0
+      raise 'This test must be run as root'
+    end
+    @name = 'test'
+    @container = LXC::Container.new(@name)
+    @container.create('ubuntu') unless @container.defined?
+    @container.start unless @container.running?
+    @container.unfreeze
+    # Destroy leftover snapshots so we don't take up the whole disk
+    @container.snapshot_list.each do |snapshot, commentfile, timestamp, snapshotfile|
+      @container.snapshot_destroy(snapshot)
+    end
+  end
+
+# TODO find a way to test unblockness for functions that don't run quite so long
+#  def test_add_device_node_allows_ruby_to_continue
+#    assert_long_running_function_does_not_block_ruby do
+#      @container.add_device_node('/dev/ttyS0')
+#    end
+#  end
+
+  def test_attach_wait_true_allows_ruby_to_continue
+    assert_long_running_function_does_not_block_ruby do
+      @container.attach(wait: true) do
+        sleep(2)
+      end
+    end
+  end
+
+  def test_clone_allows_ruby_to_continue
+    # Ensure the "cloned" VM does not exist
+    cloned_name = "#{@name}_cloned"
+    cloned_to_destroy = LXC::Container.new(cloned_name)
+    cloned_to_destroy.destroy if cloned_to_destroy.defined?
+
+    cloned = nil
+    @container.stop
+    begin
+      assert_long_running_function_does_not_block_ruby do
+        cloned = @container.clone(cloned_name)
+      end
+    ensure
+      @container.start
+    end
+    # Verify that it did something
+    assert(@container.name == @name, "original container name has changed from '#{@name}' to '#{@container.name}''")
+    assert(@container.running?, "original container is not running")
+    assert(cloned.name == cloned_name, "new container name should be '#{cloned_name}', is '#{cloned.name}'")
+    assert(cloned.init_pid != @container.init_pid, "cloned container init pid is the same as the original!  Something ain't right.")
+  end
+
+# TODO Unsure how to test this without attaching to the actual tty!  Must be a way to obtain a pipe or false tty of some sort.  Manual run reveals it works, however.
+#  def test_console_allows_ruby_to_continue
+#    assert_long_running_function_does_not_block_ruby do
+#      @container.console
+#    end
+#  end
+
+  def test_destroy_allows_ruby_to_continue
+    container = LXC::Container.new("test_destroy")
+    container.create('ubuntu') if !container.defined?
+    assert_long_running_function_does_not_block_ruby do
+      container.destroy
+    end
+    assert(!container.defined?, "Destroy did not destroy the container!")
+  end
+
+# TODO find a way to test unblockness for functions that don't run quite so long
+#  def test_freeze_allows_ruby_to_continue
+#    begin
+#      assert_long_running_function_does_not_block_ruby do
+#        @container.freeze
+#      end
+#    ensure
+#      @container.unfreeze
+#    end
+#  end
+
+# TODO find a way to test unblockness for functions that don't run quite so long
+#  def test_load_config_allows_ruby_to_continue
+#    File.open('/tmp/blah', 'w') do |file|
+#      file.write("\n\n\n\n")
+#    end
+#    begin
+#      container = LXC::Container.new('blahdeblah')
+#      assert_long_running_function_does_not_block_ruby do
+#        container.load_config('/tmp/blah')
+#      end
+#    ensure
+#      File.delete('/tmp/blah')
+#    end
+#  end
+
+# TODO find a way to test unblockness for functions that don't run quite so long
+#  def test_reboot_allows_ruby_to_continue
+#    assert_long_running_function_does_not_block_ruby do
+#      @container.reboot
+#    end
+#  end
+
+# TODO find a way to test unblockness for functions that don't run quite so long
+#  def test_remove_device_node_allows_ruby_to_continue
+#    assert_long_running_function_does_not_block_ruby do
+#      @container.remove_device_node('/dev/ttyS0')
+#    end
+#  end
+
+# TODO find a way to test unblockness for functions that don't run quite so long
+#  def test_save_config_allows_ruby_to_continue
+#    File.unlink('/tmp/blah') if File.exist?('/tmp/blah')
+#    assert_long_running_function_does_not_block_ruby do
+#      @container.save_config('/tmp/blah')
+#    end
+#    assert(File.exist?('/tmp/blah'), "save_config did not save /tmp/blah!")
+#  end
+
+  def test_shutdown_allows_ruby_to_continue
+    begin
+      assert_long_running_function_does_not_block_ruby do
+        @container.shutdown
+      end
+    ensure
+      @container.start if !@container.running?
+    end
+  end
+
+  def test_snapshot_allows_ruby_to_continue
+    @container.stop
+    begin
+      assert_long_running_function_does_not_block_ruby do
+        @container.snapshot
+      end
+    ensure
+      @container.start
+    end
+  end
+
+  def test_snapshot_destroy_allows_ruby_to_continue
+    @container.stop
+    begin
+      snapshot = @container.snapshot
+      assert_long_running_function_does_not_block_ruby do
+        @container.snapshot_destroy(snapshot)
+      end
+    ensure
+      @container.start
+    end
+  end
+
+# TODO find a way to test unblockness for functions that don't run quite so long
+#  def test_snapshot_list_allows_ruby_to_continue
+#    @container.stop
+#    begin
+#      snapshot = @container.snapshot
+#      assert_long_running_function_does_not_block_ruby do
+#        assert(@container.snapshot_list > 0, "Snapshot list was empty!"
+#      end
+#    ensure
+#      @container.start
+#    end
+#  end
+
+  def test_snapshot_restore_allows_ruby_to_continue
+    @container.stop
+    begin
+      snapshot = @container.snapshot
+      assert_long_running_function_does_not_block_ruby do
+        @container.snapshot_restore(snapshot)
+      end
+    ensure
+      @container.start
+    end
+  end
+
+  def test_start_allows_ruby_to_continue
+    @container.stop
+    assert_long_running_function_does_not_block_ruby do
+      @container.start
+    end
+    assert(@container.running?, "Start did not start container! State = #{@container.state}")
+  end
+
+# TODO find a way to test unblockness for functions that don't run quite so long
+#  def test_stop_allows_ruby_to_continue
+#    begin
+#      assert_long_running_function_does_not_block_ruby do
+#        @container.stop
+#      end
+#      assert(!@container.running?, "Stop did not stop container! State = #{@container.state}")
+#    ensure
+#      @container.start if !@container.running?
+#    end
+#  end
+
+# TODO find a way to test unblockness for functions that don't run quite so long
+#  def test_unfreeze_allows_ruby_to_continue
+#    @container.freeze
+#    assert_long_running_function_does_not_block_ruby do
+#      @container.unfreeze
+#    end
+#  end
+
+  def test_wait_allows_ruby_to_continue
+    t = Thread.new do
+      sleep(0.5)
+      @container.stop
+    end
+    begin
+      assert_long_running_function_does_not_block_ruby do
+        @container.wait(:stopped, 2)
+      end
+      assert(@container.state == :stopped, "Container never stopped! State is now #{@container.state.inspect}")
+    ensure
+      t.join
+      @container.start if !@container.running?
+    end
+  end
+end