Libguestfs - May 2023 - [PATCH libguestfs 2/2] Only leave/enter blocking_section when OCaml lock is not held

Fixes deadlocks on OCaml5 when trying to get the lock that is already
held:

Fatal error during lock: Resource deadlock avoided
---
 ocaml/guestfs-c.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ocaml/guestfs-c.c b/ocaml/guestfs-c.c
index 3888c9456..bcf8e6ab3 100644
--- a/ocaml/guestfs-c.c
+++ b/ocaml/guestfs-c.c
@@ -395,12 +395,16 @@ event_callback_wrapper (guestfs_h *g,
   /* Ensure we are holding the GC lock before any GC operations are
    * possible. (RHBZ#725824)
    */
-  caml_leave_blocking_section ();
+  bool in_blocking_section = (caml_state == NULL);
+
+  if (in_blocking_section)
+    caml_leave_blocking_section ();
 
   event_callback_wrapper_locked (g, data, event, event_handle, flags,
                                  buf, buf_len, array, array_len);
 
-  caml_enter_blocking_section ();
+  if (in_blocking_section)
+    caml_enter_blocking_section ();
 }
 
 value
-- 
2.40.1

On 5/27/23 15:35, J?rgen H?tzel wrote:
> Fixes deadlocks on OCaml5 when trying to get the lock that is already
> held:
> 
> Fatal error during lock: Resource deadlock avoided
> ---
>  ocaml/guestfs-c.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/ocaml/guestfs-c.c b/ocaml/guestfs-c.c
> index 3888c9456..bcf8e6ab3 100644
> --- a/ocaml/guestfs-c.c
> +++ b/ocaml/guestfs-c.c
> @@ -395,12 +395,16 @@ event_callback_wrapper (guestfs_h *g,
>    /* Ensure we are holding the GC lock before any GC operations are
>     * possible. (RHBZ#725824)
>     */
> -  caml_leave_blocking_section ();
> +  bool in_blocking_section = (caml_state == NULL);
> +
> +  if (in_blocking_section)
> +    caml_leave_blocking_section ();
>  
>    event_callback_wrapper_locked (g, data, event, event_handle, flags,
>                                   buf, buf_len, array, array_len);
>  
> -  caml_enter_blocking_section ();
> +  if (in_blocking_section)
> +    caml_enter_blocking_section ();
>  }
>  
>  value
I think the commit message should describe the issue and justify the
change in much more detail.

Laszlo

On Sat, May 27, 2023 at 03:35:38PM +0200, J?rgen H?tzel
wrote:
> Fixes deadlocks on OCaml5 when trying to get the lock that is already
> held:
> 
> Fatal error during lock: Resource deadlock avoided
> ---
>  ocaml/guestfs-c.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/ocaml/guestfs-c.c b/ocaml/guestfs-c.c
> index 3888c9456..bcf8e6ab3 100644
> --- a/ocaml/guestfs-c.c
> +++ b/ocaml/guestfs-c.c
> @@ -395,12 +395,16 @@ event_callback_wrapper (guestfs_h *g,
>    /* Ensure we are holding the GC lock before any GC operations are
>     * possible. (RHBZ#725824)
>     */
> -  caml_leave_blocking_section ();
> +  bool in_blocking_section = (caml_state == NULL);
> +
> +  if (in_blocking_section)
> +    caml_leave_blocking_section ();
>  
>    event_callback_wrapper_locked (g, data, event, event_handle, flags,
>                                   buf, buf_len, array, array_len);
>  
> -  caml_enter_blocking_section ();
> +  if (in_blocking_section)
> +    caml_enter_blocking_section ();
>  }
I don't understand the reason why this patch is needed.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit