Move monitor object unreference from virCHStartEventHandler
to virCHEventHandlerLoop. Put VM unreference after debug
print in virCHEventHandlerLoop.

Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com>
---
 src/ch/ch_events.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
index 1cce30836a..20e7fbd705 100644
--- a/src/ch/ch_events.c
+++ b/src/ch/ch_events.c
@@ -286,9 +286,10 @@ virCHEventHandlerLoop(void *data)
         }
     }
 
-    g_clear_pointer(&mon->event_buffer.buffer, g_free);
-    virObjectUnref(vm);
+    g_clear_pointer(&(mon->event_buffer.buffer), g_free);
     VIR_DEBUG("%s: Event handler loop thread exiting", vm->def->name);
+    virObjectUnref(vm);
+    virObjectUnref(mon);
     return;
 }
 
@@ -308,7 +309,6 @@ virCHStartEventHandler(virCHMonitor *mon)
         virObjectUnref(mon);
         return -1;
     }
-    virObjectUnref(mon);
 
     g_atomic_int_set(&mon->event_handler_stop, 0);
     return 0;
-- 
2.48.1

On Thu, Mar 06, 2025 at 11:27:07 +0100, Kirill Shchetiniuk wrote:
> Move monitor object unreference from virCHStartEventHandler
> to virCHEventHandlerLoop. Put VM unreference after debug
> print in virCHEventHandlerLoop.

Please use the commit message mainly to describe the problem you're
fixiing. You only described what you changed; not why.

> 
> Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com>
> ---
>  src/ch/ch_events.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
> index 1cce30836a..20e7fbd705 100644
> --- a/src/ch/ch_events.c
> +++ b/src/ch/ch_events.c
> @@ -286,9 +286,10 @@ virCHEventHandlerLoop(void *data)
>          }
>      }
>  
> -    g_clear_pointer(&mon->event_buffer.buffer, g_free);

Okay so ...

> -    virObjectUnref(vm);
> +    g_clear_pointer(&(mon->event_buffer.buffer), g_free);

... this was taking reference of 'mon' instead of the buffer.

>      VIR_DEBUG("%s: Event handler loop thread exiting", vm->def->name);

and this debug technically accesses 'vm' which you no longer have
reference for ...

> +    virObjectUnref(vm);
> +    virObjectUnref(mon);

But that doesn't explain why this is needed.

>      return;
>  }
>  
> @@ -308,7 +309,6 @@ virCHStartEventHandler(virCHMonitor *mon)
>          virObjectUnref(mon);

and it is weird because you do unref it here ...

>          return -1;
>      }
> -    virObjectUnref(mon);

... but remove it from here.

>  
>      g_atomic_int_set(&mon->event_handler_stop, 0);
>      return 0;

> 
> Please use the commit message mainly to describe the problem you're
> fixiing. You only described what you changed; not why.
> 
Thank you for your feedback, next time will keep it mind.

> 
> and it is weird because you do unref it here ...
> 
This unref executes in case of any failure during new thread start up, as it wont be used later, otherwise this reference is used by virCHEventHandlerLoop thread and should be unrefed when virCHEventHandlerLoop is finished.

> 
> ... but remove it from here.
>
In case of successful startup we do not want to unref an object cause it will be used in other thread.