[PATCH v3 1/2] tap-win32: cleanup leaked handles on tap close

Gal Horowitz posted 2 patches 4 months, 2 weeks ago
Maintainers: Jason Wang <jasowang@redhat.com>, Stefan Weil <sw@weilnetz.de>
There is a newer version of this series
[PATCH v3 1/2] tap-win32: cleanup leaked handles on tap close
Posted by Gal Horowitz 4 months, 2 weeks ago
Signed-off-by: Gal Horowitz <galush.horowitz@gmail.com>
---
 net/tap-win32.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/net/tap-win32.c b/net/tap-win32.c
index 38baf90e0b3f121f74eb32f1bff779c84ce03114..1a79e35c2708eae32ccdbc873908aa1ccc7a03f0 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -104,6 +104,7 @@ typedef struct tap_win32_overlapped {
     HANDLE output_queue_semaphore;
     HANDLE free_list_semaphore;
     HANDLE tap_semaphore;
+    HANDLE thread_handle;
     CRITICAL_SECTION output_queue_cs;
     CRITICAL_SECTION free_list_cs;
     OVERLAPPED read_overlapped;
@@ -604,7 +605,6 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
         unsigned long debug;
     } version;
     DWORD version_len;
-    DWORD idThread;
 
     if (preferred_name != NULL) {
         snprintf(name_buffer, sizeof(name_buffer), "%s", preferred_name);
@@ -647,13 +647,31 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
 
     tap_win32_overlapped_init(&tap_overlapped, handle);
 
+    tap_overlapped.thread_handle = CreateThread(NULL, 0,
+        tap_win32_thread_entry, (LPVOID)&tap_overlapped, 0, NULL);
+
     *phandle = &tap_overlapped;
 
-    CreateThread(NULL, 0, tap_win32_thread_entry,
-                 (LPVOID)&tap_overlapped, 0, &idThread);
     return 0;
 }
 
+static void tap_win32_close(tap_win32_overlapped_t *overlapped)
+{
+    TerminateThread(overlapped->thread_handle, 0);
+
+    CloseHandle(overlapped->tap_semaphore);
+    CloseHandle(overlapped->free_list_semaphore);
+    CloseHandle(overlapped->output_queue_semaphore);
+
+    DeleteCriticalSection(&overlapped->free_list_cs);
+    DeleteCriticalSection(&overlapped->output_queue_cs);
+
+    CloseHandle(overlapped->write_event);
+    CloseHandle(overlapped->read_event);
+
+    CloseHandle(overlapped->handle);
+}
+
 /********************************************/
 
  typedef struct TAPState {
@@ -667,9 +685,8 @@ static void tap_cleanup(NetClientState *nc)
 
     qemu_del_wait_object(s->handle->tap_semaphore, NULL, NULL);
 
-    /* FIXME: need to kill thread and close file handle:
-       tap_win32_close(s);
-    */
+    tap_win32_close(s->handle);
+    s->handle = NULL;
 }
 
 static ssize_t tap_receive(NetClientState *nc, const uint8_t *buf, size_t size)

-- 
2.34.1
Re: [PATCH v3 1/2] tap-win32: cleanup leaked handles on tap close
Posted by Jason Wang 3 months, 4 weeks ago
On Wed, Sep 24, 2025 at 10:50 PM Gal Horowitz <galush.horowitz@gmail.com> wrote:
>
> Signed-off-by: Gal Horowitz <galush.horowitz@gmail.com>
> ---
>  net/tap-win32.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/net/tap-win32.c b/net/tap-win32.c
> index 38baf90e0b3f121f74eb32f1bff779c84ce03114..1a79e35c2708eae32ccdbc873908aa1ccc7a03f0 100644
> --- a/net/tap-win32.c
> +++ b/net/tap-win32.c
> @@ -104,6 +104,7 @@ typedef struct tap_win32_overlapped {
>      HANDLE output_queue_semaphore;
>      HANDLE free_list_semaphore;
>      HANDLE tap_semaphore;
> +    HANDLE thread_handle;
>      CRITICAL_SECTION output_queue_cs;
>      CRITICAL_SECTION free_list_cs;
>      OVERLAPPED read_overlapped;
> @@ -604,7 +605,6 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
>          unsigned long debug;
>      } version;
>      DWORD version_len;
> -    DWORD idThread;
>
>      if (preferred_name != NULL) {
>          snprintf(name_buffer, sizeof(name_buffer), "%s", preferred_name);
> @@ -647,13 +647,31 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
>
>      tap_win32_overlapped_init(&tap_overlapped, handle);
>
> +    tap_overlapped.thread_handle = CreateThread(NULL, 0,
> +        tap_win32_thread_entry, (LPVOID)&tap_overlapped, 0, NULL);

Should we check the return value of CreateThread() ?

> +
>      *phandle = &tap_overlapped;
>
> -    CreateThread(NULL, 0, tap_win32_thread_entry,
> -                 (LPVOID)&tap_overlapped, 0, &idThread);
>      return 0;
>  }
>
> +static void tap_win32_close(tap_win32_overlapped_t *overlapped)
> +{
> +    TerminateThread(overlapped->thread_handle, 0);
> +
> +    CloseHandle(overlapped->tap_semaphore);
> +    CloseHandle(overlapped->free_list_semaphore);
> +    CloseHandle(overlapped->output_queue_semaphore);
> +
> +    DeleteCriticalSection(&overlapped->free_list_cs);
> +    DeleteCriticalSection(&overlapped->output_queue_cs);
> +
> +    CloseHandle(overlapped->write_event);
> +    CloseHandle(overlapped->read_event);
> +
> +    CloseHandle(overlapped->handle);
> +}
> +
>  /********************************************/
>
>   typedef struct TAPState {
> @@ -667,9 +685,8 @@ static void tap_cleanup(NetClientState *nc)
>
>      qemu_del_wait_object(s->handle->tap_semaphore, NULL, NULL);
>
> -    /* FIXME: need to kill thread and close file handle:
> -       tap_win32_close(s);
> -    */
> +    tap_win32_close(s->handle);
> +    s->handle = NULL;
>  }
>
>  static ssize_t tap_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>
> --
> 2.34.1
>

Thanks
Re: [PATCH v3 1/2] tap-win32: cleanup leaked handles on tap close
Posted by Jason Wang 3 months, 4 weeks ago
On Wed, Sep 24, 2025 at 10:50 PM Gal Horowitz <galush.horowitz@gmail.com> wrote:
>
> Signed-off-by: Gal Horowitz <galush.horowitz@gmail.com>
> ---

Please add a commit log for this and next patch. E.g what is being
fixed and why.

Thanks