[v1] iothread: fix epollfd leak in the process of delIOThread

[Qemu-devel] [PATCH] iothread: fix epollfd leak in the process of delIOThread

Posted by Jie Wang 5 years, 11 months ago

When we call addIOThread, the epollfd created in aio_context_setup,
but not close it in the process of delIOThread, so the epollfd will leak.

Signed-off-by: Jie Wang <wangjie88@huawei.com>
---
 iothread.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/iothread.c b/iothread.c
index aff1281..23ac0a8 100644
--- a/iothread.c
+++ b/iothread.c
@@ -128,6 +128,7 @@ static void iothread_instance_finalize(Object *obj)
      * GSources first before destroying any GMainContext.
      */
     if (iothread->ctx) {
+        close(iothread->ctx->epollfd);
         aio_context_unref(iothread->ctx);
         iothread->ctx = NULL;
     }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] iothread: fix epollfd leak in the process of delIOThread

Posted by Fam Zheng 5 years, 11 months ago

On Tue, 05/15 20:00, Jie Wang wrote:
> When we call addIOThread, the epollfd created in aio_context_setup,
> but not close it in the process of delIOThread, so the epollfd will leak.
> 
> Signed-off-by: Jie Wang <wangjie88@huawei.com>
> ---
>  iothread.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/iothread.c b/iothread.c
> index aff1281..23ac0a8 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -128,6 +128,7 @@ static void iothread_instance_finalize(Object *obj)
>       * GSources first before destroying any GMainContext.
>       */
>      if (iothread->ctx) {
> +        close(iothread->ctx->epollfd);
>          aio_context_unref(iothread->ctx);
>          iothread->ctx = NULL;
>      }
> -- 
> 1.8.3.1
> 

Please add an aio_context_destroy() function in aio-posix.c and call it from
aio_context_finalize(). IOThread code should not touch AioContext internals.
Also please remember to wrap the close() code in CONFIG_EPOLL_CREATE1.  An empty
function may need to be added to aio-win32.c, too.

Fam

Re: [Qemu-devel] [PATCH] iothread: fix epollfd leak in the process of delIOThread

Posted by Peter Xu 5 years, 11 months ago

On Wed, May 16, 2018 at 09:38:31AM +0800, Fam Zheng wrote:
> On Tue, 05/15 20:00, Jie Wang wrote:
> > When we call addIOThread, the epollfd created in aio_context_setup,
> > but not close it in the process of delIOThread, so the epollfd will leak.
> > 
> > Signed-off-by: Jie Wang <wangjie88@huawei.com>
> > ---
> >  iothread.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/iothread.c b/iothread.c
> > index aff1281..23ac0a8 100644
> > --- a/iothread.c
> > +++ b/iothread.c
> > @@ -128,6 +128,7 @@ static void iothread_instance_finalize(Object *obj)
> >       * GSources first before destroying any GMainContext.
> >       */
> >      if (iothread->ctx) {
> > +        close(iothread->ctx->epollfd);
> >          aio_context_unref(iothread->ctx);
> >          iothread->ctx = NULL;
> >      }
> > -- 
> > 1.8.3.1
> > 
> 
> Please add an aio_context_destroy() function in aio-posix.c and call it from
> aio_context_finalize(). IOThread code should not touch AioContext internals.

I believe Fam means aio_ctx_finalize().

> Also please remember to wrap the close() code in CONFIG_EPOLL_CREATE1.  An empty
> function may need to be added to aio-win32.c, too.

Agreed.

-- 
Peter Xu