This should not be needed, but here's what's happening:
virStrToLong_*() family of functions was switched from strtol*()
to g_ascii_strtol*() in order to handle corner cases on Windows
(most notably parsing hex numbers with base=0) - see
v9.4.0-61-g2ed41d7cd9. But what we did not realize back then, is
the fact that g_ascii_strtol*() family has their own global lock
rendering virStrToLong_*() function unsafe between fork() +
exec(). Worse, if one of the threads has to wait for the lock (or
on its corresponding condition), then errno is mangled and
g_ascii_strtol*() signals an error, even though there's no error.

Read more here:

  https://gitlab.gnome.org/GNOME/glib/-/issues/3034

Nevertheless, if we make glib init the g_ascii_strtol*() global
state (by calling one function from g_ascii_strtol*() family),
then there shouldn't be any congestion on the lock and thus no
errno mangling.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/libvirt.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/libvirt.c b/src/libvirt.c
index 2e470adf98..69d5b13bff 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -214,6 +214,14 @@ virGlobalInit(void)
     if (virErrorInitialize() < 0)
         goto error;
 
+    /* Make glib initialize its own global state. See more:
+     *
+     *   https://gitlab.gnome.org/GNOME/glib/-/issues/3034
+     *
+     * TODO: Remove ASAP.
+     */
+    g_ascii_strtoull("0", NULL, 0);
+
     virFileActivateDirOverrideForLib();
 
     if (getuid() != geteuid() ||
-- 
2.39.3

On Wed, Jun 21, 2023 at 04:09:11PM +0200, Michal Privoznik wrote:
> This should not be needed, but here's what's happening:
> virStrToLong_*() family of functions was switched from strtol*()
> to g_ascii_strtol*() in order to handle corner cases on Windows
> (most notably parsing hex numbers with base=0) - see
> v9.4.0-61-g2ed41d7cd9. But what we did not realize back then, is
> the fact that g_ascii_strtol*() family has their own global lock
> rendering virStrToLong_*() function unsafe between fork() +
> exec(). Worse, if one of the threads has to wait for the lock (or
> on its corresponding condition), then errno is mangled and
> g_ascii_strtol*() signals an error, even though there's no error.
> 
> Read more here:
> 
>   https://gitlab.gnome.org/GNOME/glib/-/issues/3034
> 
> Nevertheless, if we make glib init the g_ascii_strtol*() global
> state (by calling one function from g_ascii_strtol*() family),
> then there shouldn't be any congestion on the lock and thus no
> errno mangling.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/libvirt.c | 8 ++++++++
>  1 file changed, 8 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Wed, Jun 21, 2023 at 5:30 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Jun 21, 2023 at 04:09:11PM +0200, Michal Privoznik wrote:
> > This should not be needed, but here's what's happening:
> > virStrToLong_*() family of functions was switched from strtol*()
> > to g_ascii_strtol*() in order to handle corner cases on Windows
> > (most notably parsing hex numbers with base=0) - see
> > v9.4.0-61-g2ed41d7cd9. But what we did not realize back then, is
> > the fact that g_ascii_strtol*() family has their own global lock
> > rendering virStrToLong_*() function unsafe between fork() +
> > exec(). Worse, if one of the threads has to wait for the lock (or
> > on its corresponding condition), then errno is mangled and
> > g_ascii_strtol*() signals an error, even though there's no error.
> >
> > Read more here:
> >
> >   https://gitlab.gnome.org/GNOME/glib/-/issues/3034
> >
> > Nevertheless, if we make glib init the g_ascii_strtol*() global
> > state (by calling one function from g_ascii_strtol*() family),
> > then there shouldn't be any congestion on the lock and thus no
> > errno mangling.
> >
> > Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> > ---
> >  src/libvirt.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Thanks, let me push this one, as it fixes both problems (daedlock and
string parse problem) and work on the other two next week as I'm going
AFK for the rest of the week. Jirka plans to freeze on Tuesday next
week but after this patch, close_range()/closefrom() is not that
critical.

Michal

On Thu, Jun 22, 2023 at 11:10:25AM +0200, Michal Privoznik wrote:
> On Wed, Jun 21, 2023 at 5:30 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Wed, Jun 21, 2023 at 04:09:11PM +0200, Michal Privoznik wrote:
> > > This should not be needed, but here's what's happening:
> > > virStrToLong_*() family of functions was switched from strtol*()
> > > to g_ascii_strtol*() in order to handle corner cases on Windows
> > > (most notably parsing hex numbers with base=0) - see
> > > v9.4.0-61-g2ed41d7cd9. But what we did not realize back then, is
> > > the fact that g_ascii_strtol*() family has their own global lock
> > > rendering virStrToLong_*() function unsafe between fork() +
> > > exec(). Worse, if one of the threads has to wait for the lock (or
> > > on its corresponding condition), then errno is mangled and
> > > g_ascii_strtol*() signals an error, even though there's no error.
> > >
> > > Read more here:
> > >
> > >   https://gitlab.gnome.org/GNOME/glib/-/issues/3034
> > >
> > > Nevertheless, if we make glib init the g_ascii_strtol*() global
> > > state (by calling one function from g_ascii_strtol*() family),
> > > then there shouldn't be any congestion on the lock and thus no
> > > errno mangling.
> > >
> > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> > > ---
> > >  src/libvirt.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> >
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Thanks, let me push this one, as it fixes both problems (daedlock and
> string parse problem) and work on the other two next week as I'm going
> AFK for the rest of the week. Jirka plans to freeze on Tuesday next
> week but after this patch, close_range()/closefrom() is not that
> critical.

Sure, sounds fine to me.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|