On 10/24/24 23:30, Peter Xu wrote:
Both migration thread or background snapshot thread will take a refcount of
the migration object at the entrace of the thread function.
That makes sense, because it protects the object from being freed by the
main thread in migration_shutdown() later, but it might still race with it
if the thread is scheduled too late. Consider the case right after
pthread_create() happened, VM shuts down with the object released, but
right after that the migration thread finally got created, referencing
MigrationState* in the opaque pointer which is already freed.
The only 100% safe way to make sure it won't get freed is taking the
refcount right before the thread is created, meanwhile when BQL is held.
Signed-off-by: Peter Xu <pet...@redhat.com>
---
migration/migration.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 74812ca785..e82ffa8cf3 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3491,7 +3491,6 @@ static void *migration_thread(void *opaque)
rcu_register_thread();
- object_ref(OBJECT(s));
update_iteration_initial_status(s);
if (!multifd_send_setup()) {
@@ -3629,7 +3628,6 @@ static void *bg_migration_thread(void *opaque)
int ret;
rcu_register_thread();
- object_ref(OBJECT(s));
migration_rate_set(RATE_LIMIT_DISABLED);
@@ -3841,6 +3839,14 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
}
}
+ /*
+ * Take a refcount to make sure the migration object won't get freed by
+ * the main thread already in migration_shutdown().
+ *
+ * The refcount will be released at the end of the thread function.
+ */
+ object_ref(OBJECT(s));
+
if (migrate_background_snapshot()) {
qemu_thread_create(&s->thread, MIGRATION_THREAD_SNAPSHOT,
bg_migration_thread, s, QEMU_THREAD_JOINABLE);
yes. It is safer to take a ref before starting the migration thread.
Reviewed-by: Cédric Le Goater <c...@redhat.com>
Thanks,
C.
