[PATCH 0/3] Fix recent deadlocks when spawning processes
This is basically a v2 of: https://listman.redhat.com/archives/libvir-list/2023-June/240320.html But after discussion with Dan and most importantly, once he found the root cause we can do proper fixes instead of papering over the issue. Michal Prívozník (3): vircommand: Use closefrom() more often vircommand: Utilize close_range() virGlobalInit: Make glib init its own global state meson.build | 2 + src/libvirt.c | 8 ++ src/util/vircommand.c | 239 +++++++++++++++++++++++++++++++----------- 3 files changed, 190 insertions(+), 59 deletions(-) -- 2.39.3
As of commit v5.9-rc1~160^2~3 the Linux kernel has close_range() syscall, which closes not just one FD but whole range. Then, in its commit glibc-2.34~115 glibc introduced closefrom() which is just a wrapper over close_range(), but it allows us to use FreeBSD-only implementation on Linux too, as both OS-es now have the same function. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- meson.build | 1 + src/util/vircommand.c | 124 ++++++++++++++++++++++-------------------- 2 files changed, 66 insertions(+), 59 deletions(-) diff --git a/meson.build b/meson.build index aa391e7178..a4b52b6156 100644 --- a/meson.build +++ b/meson.build @@ -573,6 +573,7 @@ libvirt_export_dynamic = cc.first_supported_link_argument([ # check availability of various common functions (non-fatal if missing) functions = [ + 'closefrom', 'elf_aux_info', 'explicit_bzero', 'fallocate', diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 49abb53c28..b8b8d48f92 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -479,7 +479,68 @@ virExecCommon(virCommand *cmd, gid_t *groups, int ngroups) return 0; } -# ifdef __linux__ +# ifdef WITH_CLOSEFROM +# define USE_CLOSEFROM +# else +# define USE_GENERIC +# endif + + +# ifdef USE_CLOSEFROM +static int +virCommandMassClose(virCommand *cmd, + int childin, + int childout, + int childerr) +{ + int lastfd = -1; + int fd = -1; + size_t i; + + /* + * Two phases of closing. + * + * The first (inefficient) phase iterates over FDs, + * preserving certain FDs we need to pass down, and + * closing others. The number of iterations is bounded + * to the number of the biggest FD we need to preserve. + * + * The second (speedy) phase uses closefrom() to cull + * all remaining FDs in the process. + * + * Usually the first phase will be fairly quick only + * processing a handful of low FD numbers, and thus using + * closefrom() is a massive win for high ulimit() NFILES + * values. + */ + lastfd = MAX(lastfd, childin); + lastfd = MAX(lastfd, childout); + lastfd = MAX(lastfd, childerr); + + for (i = 0; i < cmd->npassfd; i++) + lastfd = MAX(lastfd, cmd->passfd[i].fd); + + for (fd = 0; fd <= lastfd; fd++) { + if (fd == childin || fd == childout || fd == childerr) + continue; + if (!virCommandFDIsSet(cmd, fd)) { + int tmpfd = fd; + VIR_MASS_CLOSE(tmpfd); + } else if (virSetInherit(fd, true) < 0) { + virReportSystemError(errno, _("failed to preserve fd %1$d"), fd); + return -1; + } + } + + closefrom(lastfd + 1); + + return 0; +} +# endif /* ! WITH_CLOSEFROM */ + + +# ifdef USE_GENERIC +# ifdef __linux__ /* On Linux, we can utilize procfs and read the table of opened * FDs and selectively close only those FDs we don't want to pass * onto child process (well, the one we will exec soon since this @@ -515,7 +576,7 @@ virCommandMassCloseGetFDsLinux(virCommand *cmd G_GNUC_UNUSED, return 0; } -# else /* !__linux__ */ +# else /* !__linux__ */ static int virCommandMassCloseGetFDsGeneric(virCommand *cmd G_GNUC_UNUSED, @@ -524,61 +585,7 @@ virCommandMassCloseGetFDsGeneric(virCommand *cmd G_GNUC_UNUSED, virBitmapSetAll(fds); return 0; } -# endif /* !__linux__ */ - -# ifdef __FreeBSD__ - -static int -virCommandMassClose(virCommand *cmd, - int childin, - int childout, - int childerr) -{ - int lastfd = -1; - int fd = -1; - size_t i; - - /* - * Two phases of closing. - * - * The first (inefficient) phase iterates over FDs, - * preserving certain FDs we need to pass down, and - * closing others. The number of iterations is bounded - * to the number of the biggest FD we need to preserve. - * - * The second (speedy) phase uses closefrom() to cull - * all remaining FDs in the process. - * - * Usually the first phase will be fairly quick only - * processing a handful of low FD numbers, and thus using - * closefrom() is a massive win for high ulimit() NFILES - * values. - */ - lastfd = MAX(lastfd, childin); - lastfd = MAX(lastfd, childout); - lastfd = MAX(lastfd, childerr); - - for (i = 0; i < cmd->npassfd; i++) - lastfd = MAX(lastfd, cmd->passfd[i].fd); - - for (fd = 0; fd <= lastfd; fd++) { - if (fd == childin || fd == childout || fd == childerr) - continue; - if (!virCommandFDIsSet(cmd, fd)) { - int tmpfd = fd; - VIR_MASS_CLOSE(tmpfd); - } else if (virSetInherit(fd, true) < 0) { - virReportSystemError(errno, _("failed to preserve fd %1$d"), fd); - return -1; - } - } - - closefrom(lastfd + 1); - - return 0; -} - -# else /* ! __FreeBSD__ */ +# endif /* !__linux__ */ static int virCommandMassClose(virCommand *cmd, @@ -628,8 +635,7 @@ virCommandMassClose(virCommand *cmd, return 0; } - -# endif /* ! __FreeBSD__ */ +# endif /* ! USE_GENERIC */ /* -- 2.39.3
On Wed, Jun 21, 2023 at 04:09:09PM +0200, Michal Privoznik wrote:
As of commit v5.9-rc1~160^2~3 the Linux kernel has close_range() syscall, which closes not just one FD but whole range. Then, in its commit glibc-2.34~115 glibc introduced closefrom() which is just a wrapper over close_range(), but it allows us to use FreeBSD-only implementation on Linux too, as both OS-es now have the same function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- meson.build | 1 + src/util/vircommand.c | 124 ++++++++++++++++++++++-------------------- 2 files changed, 66 insertions(+), 59 deletions(-)
diff --git a/meson.build b/meson.build index aa391e7178..a4b52b6156 100644 --- a/meson.build +++ b/meson.build @@ -573,6 +573,7 @@ libvirt_export_dynamic = cc.first_supported_link_argument([ # check availability of various common functions (non-fatal if missing)
functions = [ + 'closefrom', 'elf_aux_info', 'explicit_bzero', 'fallocate', diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 49abb53c28..b8b8d48f92 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -479,7 +479,68 @@ virExecCommon(virCommand *cmd, gid_t *groups, int ngroups) return 0; }
-# ifdef __linux__ +# ifdef WITH_CLOSEFROM +# define USE_CLOSEFROM +# else +# define USE_GENERIC +# endif + + +# ifdef USE_CLOSEFROM +static int +virCommandMassClose(virCommand *cmd, + int childin, + int childout, + int childerr) +{ + int lastfd = -1; + int fd = -1; + size_t i; + + /* + * Two phases of closing. + * + * The first (inefficient) phase iterates over FDs, + * preserving certain FDs we need to pass down, and + * closing others. The number of iterations is bounded + * to the number of the biggest FD we need to preserve. + * + * The second (speedy) phase uses closefrom() to cull + * all remaining FDs in the process. + * + * Usually the first phase will be fairly quick only + * processing a handful of low FD numbers, and thus using + * closefrom() is a massive win for high ulimit() NFILES + * values. + */ + lastfd = MAX(lastfd, childin); + lastfd = MAX(lastfd, childout); + lastfd = MAX(lastfd, childerr); + + for (i = 0; i < cmd->npassfd; i++) + lastfd = MAX(lastfd, cmd->passfd[i].fd); + + for (fd = 0; fd <= lastfd; fd++) { + if (fd == childin || fd == childout || fd == childerr) + continue; + if (!virCommandFDIsSet(cmd, fd)) { + int tmpfd = fd; + VIR_MASS_CLOSE(tmpfd); + } else if (virSetInherit(fd, true) < 0) { + virReportSystemError(errno, _("failed to preserve fd %1$d"), fd); + return -1; + } + } + + closefrom(lastfd + 1);
GLibC might have closefrom() but be in a container running on an older kernel that lacks close_range syscall. We could ignore return value on FreeBSD, but now on Linux we must check the return value and fallback to the old codepath(s). 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 :|
As of commit v5.9-rc1~160^2~3 the Linux kernel has close_range() syscall, which closes not just one FD but whole range. In glibc this is exposed by automatically generated wrapper of the same name. In musl, this is not exposed, yet, but we can call the syscall() directly. In either case, we have to deal with a situation, when the kernel we're running under does not have the syscall as glibc deliberately does not implement fallback. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- meson.build | 1 + src/util/vircommand.c | 117 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/meson.build b/meson.build index a4b52b6156..ecfc1b6bdf 100644 --- a/meson.build +++ b/meson.build @@ -573,6 +573,7 @@ libvirt_export_dynamic = cc.first_supported_link_argument([ # check availability of various common functions (non-fatal if missing) functions = [ + 'close_range', 'closefrom', 'elf_aux_info', 'explicit_bzero', diff --git a/src/util/vircommand.c b/src/util/vircommand.c index b8b8d48f92..e826f5f348 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -479,13 +479,128 @@ virExecCommon(virCommand *cmd, gid_t *groups, int ngroups) return 0; } -# ifdef WITH_CLOSEFROM +# if defined(WITH_CLOSE_RANGE) || \ + (defined(WITH_SYS_SYSCALL_H) && defined(SYS_close_range)) +# define USE_CLOSE_RANGE +# elif defined(WITH_CLOSEFROM) # define USE_CLOSEFROM # else # define USE_GENERIC # endif +# ifdef USE_CLOSE_RANGE +static int +virCloseRange(unsigned int first, + unsigned int last, + unsigned int flags) +{ + + static virTristateBool has_close_range = VIR_TRISTATE_BOOL_ABSENT; + int fd; + int ret = -1; + + VIR_DEBUG("first=%u, last=%u, flags=0x%x, has_close_range=%d", + first, last, flags, has_close_range); + + if (has_close_range != VIR_TRISTATE_BOOL_NO) { +# if WITH_CLOSE_RANGE + ret = close_range(first, last, flags); +# else + ret = syscall(SYS_close_range, first, last, flags); +# endif + } + + if (ret == 0) { + if (has_close_range == VIR_TRISTATE_BOOL_ABSENT) + has_close_range = VIR_TRISTATE_BOOL_YES; + return 0; + } + + if (errno == ENOSYS) { + if (has_close_range == VIR_TRISTATE_BOOL_ABSENT) { + VIR_DEBUG("Kernel does not support close_range, falling back to naive implementation"); + has_close_range = VIR_TRISTATE_BOOL_NO; + } + } else { + return ret; + } + + /* glibc does not implement fallback, we have to implement it ourselves. */ + if (flags != 0) { + errno = EINVAL; + return -1; + } + + for (fd = first; fd <= last; fd++) { + int tmpfd = fd; + VIR_MASS_CLOSE(tmpfd); + } + + return 0; +} + + +static int +virCommandMassClose(virCommand *cmd, + int childin, + int childout, + int childerr) +{ + g_autoptr(virBitmap) fds = virBitmapNew(3); + ssize_t first; + ssize_t last; + int openmax = sysconf(_SC_OPEN_MAX); + size_t i; + + virBitmapSetBitExpand(fds, childin); + virBitmapSetBitExpand(fds, childout); + virBitmapSetBitExpand(fds, childerr); + + for (i = 0; i < cmd->npassfd; i++) { + int fd = cmd->passfd[i].fd; + + virBitmapSetBitExpand(fds, fd); + + if (virSetInherit(fd, true) < 0) { + virReportSystemError(errno, _("failed to preserve fd %1$d"), fd); + return -1; + } + } + + first = 2; + while ((last = virBitmapNextSetBit(fds, first)) >= 0) { + if (first + 1 == last) { + first = last; + continue; + } + + /* Preserve @first and @last and close everything in between. */ + if (virCloseRange(first + 1, last - 1, 0) < 0) { + virReportSystemError(errno, + _("Unable to mass close FDs (first=%1$zd, last=%2$zd)"), + first + 1, last - 1); + return -1; + } + + first = last; + } + + if (openmax < 0) + openmax = INT_MAX; + + if (virCloseRange(first + 1, openmax, 0) < 0) { + virReportSystemError(errno, + _("Unable to mass close FDs (first=%1$zd, last=%2$d"), + first + 1, openmax); + return -1; + } + + return 0; +} +# endif /* USE_CLOSE_RANGE */ + + # ifdef USE_CLOSEFROM static int virCommandMassClose(virCommand *cmd, -- 2.39.3
On Wed, Jun 21, 2023 at 04:09:10PM +0200, Michal Privoznik wrote:
As of commit v5.9-rc1~160^2~3 the Linux kernel has close_range() syscall, which closes not just one FD but whole range. In glibc this is exposed by automatically generated wrapper of the same name. In musl, this is not exposed, yet, but we can call the syscall() directly. In either case, we have to deal with a situation, when the kernel we're running under does not have the syscall as glibc deliberately does not implement fallback.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- meson.build | 1 + src/util/vircommand.c | 117 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 117 insertions(+), 1 deletion(-)
diff --git a/meson.build b/meson.build index a4b52b6156..ecfc1b6bdf 100644 --- a/meson.build +++ b/meson.build @@ -573,6 +573,7 @@ libvirt_export_dynamic = cc.first_supported_link_argument([ # check availability of various common functions (non-fatal if missing)
functions = [ + 'close_range', 'closefrom', 'elf_aux_info', 'explicit_bzero', diff --git a/src/util/vircommand.c b/src/util/vircommand.c index b8b8d48f92..e826f5f348 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -479,13 +479,128 @@ virExecCommon(virCommand *cmd, gid_t *groups, int ngroups) return 0; }
-# ifdef WITH_CLOSEFROM +# if defined(WITH_CLOSE_RANGE) || \ + (defined(WITH_SYS_SYSCALL_H) && defined(SYS_close_range)) +# define USE_CLOSE_RANGE +# elif defined(WITH_CLOSEFROM) # define USE_CLOSEFROM # else # define USE_GENERIC # endif
Although you've optimized to take advantage of using close_range to deal with gaps, in the interests of code brevity, I wonder if we shouldn't simplify this patch down to merely: #ifndef WITH_CLOSEFROM # ifdef WITH_CLOSE_RANGE # define closefrom(n) closerange(n, ~0U, 0) # define WITH_CLOSEFROM # else if defined(WITH_SYS_SYSCALL_H) && defined(SYS_close_range) # define closefrom(n) syscall(SYS_close_range, n, ~0U, 0) # define WITH_CLOSEFROM # endif #endif thus just using the existing closefrom() impl (assuming you adjust the previous patch to check return status for ENOSYS and do legacy fallback. Not convinced it is worth worrying about the FD gaps ? 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 04:40:10PM +0100, Daniel P. Berrangé wrote:
On Wed, Jun 21, 2023 at 04:09:10PM +0200, Michal Privoznik wrote:
As of commit v5.9-rc1~160^2~3 the Linux kernel has close_range() syscall, which closes not just one FD but whole range. In glibc this is exposed by automatically generated wrapper of the same name. In musl, this is not exposed, yet, but we can call the syscall() directly. In either case, we have to deal with a situation, when the kernel we're running under does not have the syscall as glibc deliberately does not implement fallback.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- meson.build | 1 + src/util/vircommand.c | 117 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 117 insertions(+), 1 deletion(-)
diff --git a/meson.build b/meson.build index a4b52b6156..ecfc1b6bdf 100644 --- a/meson.build +++ b/meson.build @@ -573,6 +573,7 @@ libvirt_export_dynamic = cc.first_supported_link_argument([ # check availability of various common functions (non-fatal if missing)
functions = [ + 'close_range', 'closefrom', 'elf_aux_info', 'explicit_bzero', diff --git a/src/util/vircommand.c b/src/util/vircommand.c index b8b8d48f92..e826f5f348 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -479,13 +479,128 @@ virExecCommon(virCommand *cmd, gid_t *groups, int ngroups) return 0; }
-# ifdef WITH_CLOSEFROM +# if defined(WITH_CLOSE_RANGE) || \ + (defined(WITH_SYS_SYSCALL_H) && defined(SYS_close_range)) +# define USE_CLOSE_RANGE +# elif defined(WITH_CLOSEFROM) # define USE_CLOSEFROM # else # define USE_GENERIC # endif
Although you've optimized to take advantage of using close_range to deal with gaps, in the interests of code brevity, I wonder if we shouldn't simplify this patch down to merely:
#ifndef WITH_CLOSEFROM # ifdef WITH_CLOSE_RANGE # define closefrom(n) closerange(n, ~0U, 0) # define WITH_CLOSEFROM # else if defined(WITH_SYS_SYSCALL_H) && defined(SYS_close_range) # define closefrom(n) syscall(SYS_close_range, n, ~0U, 0) # define WITH_CLOSEFROM # endif #endif
thus just using the existing closefrom() impl (assuming you adjust the previous patch to check return status for ENOSYS and do legacy fallback.
Not convinced it is worth worrying about the FD gaps ?
Changing my mind on this. I learnt that FreeBSD added support for close_range() to match Linux, since it was considered more flexible than their previous closefrom(). This got into FreeBSD in Apr 2020, and IIUC they backported it to 12.2, though worst case it is in 12.3 Given our supported platforms matrix I think we can assume any FreeBSD we target has close_range. IOW, we don't need to support closefrom() on FreeBSD any more. Thus I'd suggest we exclusively use close_range in livirt for both Linux and FreeBSD in the fast path, henceforth. 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 :|
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 :|
participants (2)
-
Daniel P. Berrangé -
Michal Privoznik