[PATCH 0/2] rcu: add uninit destructor for rcu so the pending calls be called before the process exit

[PATCH 0/2] rcu: add uninit destructor for rcu so the pending calls be called before the process exit

[Yonggang Luo]

This is necessary if the pending calls are close and removing temp files
Also fixes test-logging.c on msys2/mingw, on windows if the file doesn't clos=
ed, you can not remove i

Yonggang Luo (2):
  logging: Fixes memory leak in test-logging.c
  rcu: add uninit destructor for rcu so the pending calls be called
    before the process exit

 include/qemu/rcu.h   |  5 +++++
 tests/test-logging.c |  4 +++-
 util/rcu.c           | 28 ++++++++++++++++++++++++----
 3 files changed, 32 insertions(+), 5 deletions(-)

--=20
2.28.0.windows.1

[PATCH 1/2] logging: Fixes memory leak in test-logging.c

[Yonggang Luo]

g_dir_make_tmp Returns the actual name used. This string should be freed with g_free() when not needed any longer
 and is is in the GLib file name encoding. In case of errors, NULL is returned and error will be set.
Use g_autofree to free it properly

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 tests/test-logging.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test-logging.c b/tests/test-logging.c
index 8a1161de1d..957f6c08cd 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -196,7 +196,7 @@ static void rmdir_full(gchar const *root)
 
 int main(int argc, char **argv)
 {
-    gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL);
+    g_autofree gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL);
     int rc;
 
     g_test_init(&argc, &argv, NULL);
-- 
2.28.0.windows.1

[PATCH 2/2] rcu: add uninit destructor for rcu so the pending calls be called before the process exit

[Yonggang Luo]

This is necessary if the pending calls are close and removing temp files
Also fixes test-logging.c on msys2/mingw, on windows if the file doesn't closed, you can not remove it

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 include/qemu/rcu.h   |  5 +++++
 tests/test-logging.c |  2 ++
 util/rcu.c           | 28 ++++++++++++++++++++++++----
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h
index 570aa603eb..dd0a92c1d0 100644
--- a/include/qemu/rcu.h
+++ b/include/qemu/rcu.h
@@ -124,6 +124,11 @@ extern void rcu_unregister_thread(void);
 extern void rcu_enable_atfork(void);
 extern void rcu_disable_atfork(void);
 
+/*
+ * Wait all rcu call executed and exit the rcu thread.
+ */
+extern void rcu_wait_finished(void);
+
 struct rcu_head;
 typedef void RCUCBFunc(struct rcu_head *head);
 
diff --git a/tests/test-logging.c b/tests/test-logging.c
index 957f6c08cd..7a5b59f4a5 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -210,6 +210,8 @@ int main(int argc, char **argv)
                          tmp_path, test_logfile_lock);
 
     rc = g_test_run();
+    qemu_log_close();
+    rcu_wait_finished();
 
     rmdir_full(tmp_path);
     g_free(tmp_path);
diff --git a/util/rcu.c b/util/rcu.c
index 60a37f72c3..3d5ba695a4 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -234,6 +234,7 @@ retry:
 
 static void *call_rcu_thread(void *opaque)
 {
+    int *rcu_finished_ptr = (int *)opaque;
     struct rcu_head *node;
 
     rcu_register_thread();
@@ -241,6 +242,10 @@ static void *call_rcu_thread(void *opaque)
     for (;;) {
         int tries = 0;
         int n = atomic_read(&rcu_call_count);
+        if (n == 0 && atomic_mb_read(rcu_finished_ptr) == 1)
+        {
+            return NULL;
+        }
 
         /* Heuristically wait for a decent number of callbacks to pile up.
          * Fetch rcu_call_count now, we only must process elements that were
@@ -308,10 +313,12 @@ void rcu_unregister_thread(void)
     qemu_mutex_unlock(&rcu_registry_lock);
 }
 
+static QemuThread rcu_thread;
+static int rcu_finished = 0;
+
 static void rcu_init_complete(void)
 {
-    QemuThread thread;
-
+    atomic_mb_set(&rcu_finished, 0);
     qemu_mutex_init(&rcu_registry_lock);
     qemu_mutex_init(&rcu_sync_lock);
     qemu_event_init(&rcu_gp_event, true);
@@ -321,12 +328,20 @@ static void rcu_init_complete(void)
     /* The caller is assumed to have iothread lock, so the call_rcu thread
      * must have been quiescent even after forking, just recreate it.
      */
-    qemu_thread_create(&thread, "call_rcu", call_rcu_thread,
-                       NULL, QEMU_THREAD_DETACHED);
+    qemu_thread_create(&rcu_thread, "call_rcu", call_rcu_thread,
+                       &rcu_finished, QEMU_THREAD_JOINABLE);
 
     rcu_register_thread();
 }
 
+void rcu_wait_finished(void)
+{
+    if (atomic_xchg(&rcu_finished, 1) == 0)
+    {
+        qemu_thread_join(&rcu_thread);
+    }
+}
+
 static int atfork_depth = 1;
 
 void rcu_enable_atfork(void)
@@ -379,3 +394,8 @@ static void __attribute__((__constructor__)) rcu_init(void)
 #endif
     rcu_init_complete();
 }
+
+static void __attribute__((__destructor__)) rcu_uninit(void)
+{
+    rcu_wait_finished();
+}
-- 
2.28.0.windows.1

Re: [PATCH 1/2] logging: Fixes memory leak in test-logging.c

[Philippe Mathieu-Daudé]

On 9/8/20 2:10 PM, Yonggang Luo wrote:
> g_dir_make_tmp Returns the actual name used. This string should be freed with g_free() when not needed any longer
>  and is is in the GLib file name encoding. In case of errors, NULL is returned and error will be set.
> Use g_autofree to free it properly

Please limit your lines to 80 chars, see CODING_STYLE.rst:

  Line width
  ==========

  Lines should be 80 characters; try not to make them longer.

Also it is recommended to limit commit description to
72 chars. With that addressed:

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

(and please keep the reviewers tags when you repost the
same patch, this way we don't have to review your patches
twice).

> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  tests/test-logging.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/test-logging.c b/tests/test-logging.c
> index 8a1161de1d..957f6c08cd 100644
> --- a/tests/test-logging.c
> +++ b/tests/test-logging.c
> @@ -196,7 +196,7 @@ static void rmdir_full(gchar const *root)
>  
>  int main(int argc, char **argv)
>  {
> -    gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL);
> +    g_autofree gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL);
>      int rc;
>  
>      g_test_init(&argc, &argv, NULL);
>