Libguestfs - May 2014 - Re: [PATCH] generator: c.ml - wrap non deamon function with recursive mutex

From: Maros Zatko <mzatko@redhat.com>

Patch adds recursive mutex around C calls, which tries to fix
potential race condition in guestfs_umount_local (see [1]).

[1] - https://bugzilla.redhat.com/show_bug.cgi?id=917706

Maros Zatko (1):
  generator: c.ml - wrap non deamon function with recursive mutex

 generator/c.ml | 12 ++++++++++++
 1 file changed, 12 insertions(+)

-- 
1.8.5.3

From: Maros Zatko <mzatko@redhat.com>

---
 generator/c.ml | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/generator/c.ml b/generator/c.ml
index ee276dc..4333bb8 100644
--- a/generator/c.ml
+++ b/generator/c.ml
@@ -788,6 +788,9 @@ and generate_internal_actions_h ()    pr "#define
GUESTFS_INTERNAL_ACTIONS_H_\n";
   pr "\n";
 
+  pr "#include \"glthread/lock.h\"\n";
+  pr "\n";
+
   List.iter (
     fun { c_name = c_name; style = style } ->
       generate_prototype ~single_line:true ~newline:true ~handle:"g"
@@ -796,6 +799,10 @@ and generate_internal_actions_h ()    )
non_daemon_functions;
 
   pr "\n";
+
+  pr "gl_recursive_lock_define_initialized(static, global_lock)\n";
+  pr "\n";
+
   pr "#endif /* GUESTFS_INTERNAL_ACTIONS_H_ */\n"
 
 (* Generate guestfs-internal-frontend-cleanups.h file. *)
@@ -1567,6 +1574,8 @@ and generate_client_actions hash ()          c_name style;
     pr "{\n";
 
+    pr "  gl_recursive_lock_lock (global_lock);\n";
+
     handle_null_optargs optargs c_name;
 
     pr "  int trace_flag = g->trace;\n";
@@ -1617,6 +1626,9 @@ and generate_client_actions hash ()        trace_return
name style "r";
     );
     pr "\n";
+
+    pr "  gl_recursive_lock_unlock (global_lock);\n";
+
     pr "  return r;\n";
     pr "}\n";
     pr "\n"
-- 
1.8.5.3

On Thu, May 15, 2014 at 05:39:08PM +0200, mzatko@redhat.com
wrote:
> +
> +  pr "gl_recursive_lock_define_initialized(static,
global_lock)\n";
static?

I suspect this only allows one libguestfs handle per process into the
critical section at once.  I think the lock needs to be per-handle
unless I'm misunderstanding what this is for.

Rich.
> +  pr "\n";
> +
>    pr "#endif /* GUESTFS_INTERNAL_ACTIONS_H_ */\n"
>  
>  (* Generate guestfs-internal-frontend-cleanups.h file. *)
> @@ -1567,6 +1574,8 @@ and generate_client_actions hash () >         
c_name style;
>      pr "{\n";
>  
> +    pr "  gl_recursive_lock_lock (global_lock);\n";
> +
>      handle_null_optargs optargs c_name;
>  
>      pr "  int trace_flag = g->trace;\n";
> @@ -1617,6 +1626,9 @@ and generate_client_actions hash () >       
trace_return name style "r";
>      );
>      pr "\n";
> +
> +    pr "  gl_recursive_lock_unlock (global_lock);\n";
> +
>      pr "  return r;\n";
>      pr "}\n";
>      pr "\n"
> -- 
> 1.8.5.3
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html