The main problem that if we call object_new_with_class() and then
object_unref(), it fails. First of all, this is due to the fact that
finalize expects that net/colo-compare.c:colo_compare_complete() has
been called before.
On 3/30/22 17:54, Vladimir Sementsov-Ogievskiy wrote:
> 29.03.2022 00:15, Maxim Davydov wrote:
>> Fixes some possible issues with finalization. For example, finalization
>> immediately after instance_init fails on the assert.
>>
>> Signed-off-by: Maxim Davydov <maxim.davydov@openvz.org>
>> ---
>> net/colo-compare.c | 25 ++++++++++++++++---------
>> 1 file changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index 62554b5b3c..81d8de0aaa 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -1426,7 +1426,7 @@ static void colo_compare_finalize(Object *obj)
>> break;
>> }
>> }
>> - if (QTAILQ_EMPTY(&net_compares)) {
if colo_compare_active == false, event_mtx and event_complete_cond
didn't inited in colo_compare_complete()
>> + if (QTAILQ_EMPTY(&net_compares) && colo_compare_active) {
>> colo_compare_active = false;
>> qemu_mutex_destroy(&event_mtx);
>> qemu_cond_destroy(&event_complete_cond);
>> @@ -1442,19 +1442,26 @@ static void colo_compare_finalize(Object *obj)
>> colo_compare_timer_del(s);
>> - qemu_bh_delete(s->event_bh);
s->event_bh wasn't allocated in colo_compare_iothread() in
colo_compare_complete()
>> + if (s->event_bh) {
>> + qemu_bh_delete(s->event_bh);
>> + }
>> - AioContext *ctx = iothread_get_aio_context(s->iothread);
>> - aio_context_acquire(ctx);
>> - AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
>> - if (s->notify_dev) {
>> - AIO_WAIT_WHILE(ctx, !s->notify_sendco.done);
s->iothread == NULL after .instance_init (it can be detected in
colo_compare_complete(), if it has been called)
>> + if (s->iothread) {
>> + AioContext *ctx = iothread_get_aio_context(s->iothread);
>> + aio_context_acquire(ctx);
>> + AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
>> + if (s->notify_dev) {
>> + AIO_WAIT_WHILE(ctx, !s->notify_sendco.done);
>> + }
>> + aio_context_release(ctx);
>> }
>> - aio_context_release(ctx);
>> /* Release all unhandled packets after compare thead exited */
>> g_queue_foreach(&s->conn_list, colo_flush_packets, s);
>> - AIO_WAIT_WHILE(NULL, !s->out_sendco.done);
In normal situation, it flushes all packets and sets s->out_sendco.done
= true via compare_chr_send (we wait this event). But s->conn_list isn't
initialized, s->out_sendco.done == false and won't become true. So, it's
infinite waiting.
>> + /* Without colo_compare_complete done == false without packets */
>> + if (!g_queue_is_empty(&s->out_sendco.send_list)) {
>> + AIO_WAIT_WHILE(NULL, !s->out_sendco.done);
>> + }
>
> I think, would be good to add more description for this last change.
> It's not as obvious as previous two changes.
>
>> g_queue_clear(&s->conn_list);
>> g_queue_clear(&s->out_sendco.send_list);
>
>
--
Best regards,
Maxim Davydov