[PATCH] util/log: Close per-thread log file on thread termination

Greg Kurz posted 1 patch 1 year, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20221021105734.555797-1-groug@kaod.org
util/log.c | 9 +++++++++
1 file changed, 9 insertions(+)
[PATCH] util/log: Close per-thread log file on thread termination
Posted by Greg Kurz 1 year, 6 months ago
When `-D ${logfile} -d tid` is passed, qemu_log_trylock() creates
a dedicated log file for the current thread and opens it. The
corresponding file descriptor is cached in a __thread variable.
Nothing is done to close the corresponding file descriptor when the
thread terminates though and the file descriptor is leaked.

The issue was found during code inspection and reproduced manually.

Fix that with an atexit notifier.

Fixes: 4e51069d6793 ("util/log: Support per-thread log files")
Cc: richard.henderson@linaro.org
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 util/log.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/util/log.c b/util/log.c
index d6eb0378c3a3..39866bdaf2fa 100644
--- a/util/log.c
+++ b/util/log.c
@@ -42,6 +42,7 @@ static QemuMutex global_mutex;
 static char *global_filename;
 static FILE *global_file;
 static __thread FILE *thread_file;
+static __thread Notifier qemu_log_thread_cleanup_notifier;
 
 int qemu_loglevel;
 static bool log_append;
@@ -77,6 +78,12 @@ static int log_thread_id(void)
 #endif
 }
 
+static void qemu_log_thread_cleanup(Notifier *n, void *unused)
+{
+    fclose(thread_file);
+    thread_file = NULL;
+}
+
 /* Lock/unlock output. */
 
 FILE *qemu_log_trylock(void)
@@ -93,6 +100,8 @@ FILE *qemu_log_trylock(void)
                 return NULL;
             }
             thread_file = logfile;
+            qemu_log_thread_cleanup_notifier.notify = qemu_log_thread_cleanup;
+            qemu_thread_atexit_add(&qemu_log_thread_cleanup_notifier);
         } else {
             rcu_read_lock();
             /*
-- 
2.37.3
Re: [PATCH] util/log: Close per-thread log file on thread termination
Posted by Greg Kurz 1 year, 6 months ago
Cc'ing stable

On Fri, 21 Oct 2022 12:57:34 +0200
Greg Kurz <groug@kaod.org> wrote:

> When `-D ${logfile} -d tid` is passed, qemu_log_trylock() creates
> a dedicated log file for the current thread and opens it. The
> corresponding file descriptor is cached in a __thread variable.
> Nothing is done to close the corresponding file descriptor when the
> thread terminates though and the file descriptor is leaked.
> 
> The issue was found during code inspection and reproduced manually.
> 
> Fix that with an atexit notifier.
> 
> Fixes: 4e51069d6793 ("util/log: Support per-thread log files")
> Cc: richard.henderson@linaro.org
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  util/log.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/util/log.c b/util/log.c
> index d6eb0378c3a3..39866bdaf2fa 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -42,6 +42,7 @@ static QemuMutex global_mutex;
>  static char *global_filename;
>  static FILE *global_file;
>  static __thread FILE *thread_file;
> +static __thread Notifier qemu_log_thread_cleanup_notifier;
>  
>  int qemu_loglevel;
>  static bool log_append;
> @@ -77,6 +78,12 @@ static int log_thread_id(void)
>  #endif
>  }
>  
> +static void qemu_log_thread_cleanup(Notifier *n, void *unused)
> +{
> +    fclose(thread_file);
> +    thread_file = NULL;
> +}
> +
>  /* Lock/unlock output. */
>  
>  FILE *qemu_log_trylock(void)
> @@ -93,6 +100,8 @@ FILE *qemu_log_trylock(void)
>                  return NULL;
>              }
>              thread_file = logfile;
> +            qemu_log_thread_cleanup_notifier.notify = qemu_log_thread_cleanup;
> +            qemu_thread_atexit_add(&qemu_log_thread_cleanup_notifier);
>          } else {
>              rcu_read_lock();
>              /*
Re: [PATCH] util/log: Close per-thread log file on thread termination
Posted by Paolo Bonzini 1 year, 6 months ago
Queued, thanks.

Paolo