[Spice-devel] qxl-wddm-dod: Fix memory leaks

Submitted by Yuri Benditovich on May 5, 2017, 10:10 a.m.

Details

Message ID 1493979004-19344-1-git-send-email-yuri.benditovich@daynix.com
State New
Headers show
Series "qxl-wddm-dod: Fix memory leaks" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Yuri Benditovich May 5, 2017, 10:10 a.m.
The patch fixes memory leaks in previous patch
'Synchronize display mode change and pushing drawables'.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 qxldod/QxlDod.cpp | 25 ++++++++++++++++++-------
 qxldod/QxlDod.h   |  1 +
 2 files changed, 19 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
index 5168941..07246fa 100755
--- a/qxldod/QxlDod.cpp
+++ b/qxldod/QxlDod.cpp
@@ -3937,6 +3937,8 @@  QxlDevice::ExecutePresentDisplayOnly(
                 delayed += n;
                 if (currentGeneration == m_DrawGeneration)
                     PushDrawable(pDrawables[i]);
+                else
+                    DiscardDrawable(pDrawables[i]);
             }
         }
         delete[] pDrawables;
@@ -3945,6 +3947,8 @@  QxlDevice::ExecutePresentDisplayOnly(
         }
     });
     if (!operation) {
+        MmUnlockPages(ctx->Mdl);
+        IoFreeMdl(ctx->Mdl);
         delete[] pDrawables;
         return STATUS_NO_MEMORY;
     }
@@ -4556,6 +4560,19 @@  BOOLEAN QxlDevice::AttachNewBitmap(QXLDrawable *drawable, UINT8 *src, UINT8 *src
     return TRUE;
 }
 
+void QxlDevice::DiscardDrawable(QXLDrawable *drawable)
+{
+    PAGED_CODE();
+    PLIST_ENTRY pDelayedList = DelayedList(drawable);
+    // if some delayed chunks were allocated, free them
+    while (!IsListEmpty(pDelayedList)) {
+        DelayedChunk *pdc = (DelayedChunk *)RemoveHeadList(pDelayedList);
+        delete[] reinterpret_cast<BYTE*>(pdc);
+    }
+    ReleaseOutput(drawable->release_info.id);
+    DbgPrint(TRACE_LEVEL_WARNING, ("%s\n", __FUNCTION__));
+}
+
 QXLDrawable *QxlDevice::PrepareBltBits (
     BLT_INFO* pDst,
     CONST BLT_INFO* pSrc,
@@ -4605,13 +4622,7 @@  QXLDrawable *QxlDevice::PrepareBltBits (
     src += pSrc->Pitch * (height - 1);
 
     if (!AttachNewBitmap(drawable, src, src_end, (INT)pSrc->Pitch, !g_bSupportVSync)) {
-        PLIST_ENTRY pDelayedList = DelayedList(drawable);
-        // if some delayed chunks were allocated, free them
-        while (!IsListEmpty(pDelayedList)) {
-            DelayedChunk *pdc = (DelayedChunk *)RemoveHeadList(pDelayedList);
-            delete[] reinterpret_cast<BYTE*>(pdc);
-        }
-        ReleaseOutput(drawable->release_info.id);
+        DiscardDrawable(drawable);
         drawable = NULL;
     } else {
         DbgPrint(TRACE_LEVEL_INFORMATION, ("%s drawable= %p type = %d, effect = %d Dest right(%d) left(%d) top(%d) bottom(%d) src_bitmap= %p.\n", __FUNCTION__,
diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
index 2ce3ce5..77617b9 100755
--- a/qxldod/QxlDod.h
+++ b/qxldod/QxlDod.h
@@ -620,6 +620,7 @@  private:
                             size_t alloc_size, PLIST_ENTRY pDelayed);
     QXLDataChunk *MakeChunk(DelayedChunk *pdc);
     ULONG PrepareDrawable(QXLDrawable*& drawable);
+    void DiscardDrawable(QXLDrawable *drawable);
     void AsyncIo(UCHAR  Port, UCHAR Value);
     void SyncIo(UCHAR  Port, UCHAR Value);
     NTSTATUS UpdateChildStatus(BOOLEAN connect);

Comments

> 
> The patch fixes memory leaks in previous patch
> 'Synchronize display mode change and pushing drawables'.
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>

Acked.

Most of this patch was based on the patch "Move code for discarding drawable
to separate procedure" you sent couple of week. I rebased on top of it
and merged the leaks left. The results is the same, I proposed a new version.

Frediano


> ---
>  qxldod/QxlDod.cpp | 25 ++++++++++++++++++-------
>  qxldod/QxlDod.h   |  1 +
>  2 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index 5168941..07246fa 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -3937,6 +3937,8 @@ QxlDevice::ExecutePresentDisplayOnly(
>                  delayed += n;
>                  if (currentGeneration == m_DrawGeneration)
>                      PushDrawable(pDrawables[i]);
> +                else
> +                    DiscardDrawable(pDrawables[i]);
>              }
>          }
>          delete[] pDrawables;
> @@ -3945,6 +3947,8 @@ QxlDevice::ExecutePresentDisplayOnly(
>          }
>      });
>      if (!operation) {
> +        MmUnlockPages(ctx->Mdl);
> +        IoFreeMdl(ctx->Mdl);
>          delete[] pDrawables;
>          return STATUS_NO_MEMORY;
>      }
> @@ -4556,6 +4560,19 @@ BOOLEAN QxlDevice::AttachNewBitmap(QXLDrawable
> *drawable, UINT8 *src, UINT8 *src
>      return TRUE;
>  }
>  
> +void QxlDevice::DiscardDrawable(QXLDrawable *drawable)
> +{
> +    PAGED_CODE();
> +    PLIST_ENTRY pDelayedList = DelayedList(drawable);
> +    // if some delayed chunks were allocated, free them
> +    while (!IsListEmpty(pDelayedList)) {
> +        DelayedChunk *pdc = (DelayedChunk *)RemoveHeadList(pDelayedList);
> +        delete[] reinterpret_cast<BYTE*>(pdc);
> +    }
> +    ReleaseOutput(drawable->release_info.id);
> +    DbgPrint(TRACE_LEVEL_WARNING, ("%s\n", __FUNCTION__));
> +}
> +
>  QXLDrawable *QxlDevice::PrepareBltBits (
>      BLT_INFO* pDst,
>      CONST BLT_INFO* pSrc,
> @@ -4605,13 +4622,7 @@ QXLDrawable *QxlDevice::PrepareBltBits (
>      src += pSrc->Pitch * (height - 1);
>  
>      if (!AttachNewBitmap(drawable, src, src_end, (INT)pSrc->Pitch,
>      !g_bSupportVSync)) {
> -        PLIST_ENTRY pDelayedList = DelayedList(drawable);
> -        // if some delayed chunks were allocated, free them
> -        while (!IsListEmpty(pDelayedList)) {
> -            DelayedChunk *pdc = (DelayedChunk
> *)RemoveHeadList(pDelayedList);
> -            delete[] reinterpret_cast<BYTE*>(pdc);
> -        }
> -        ReleaseOutput(drawable->release_info.id);
> +        DiscardDrawable(drawable);
>          drawable = NULL;
>      } else {
>          DbgPrint(TRACE_LEVEL_INFORMATION, ("%s drawable= %p type = %d,
>          effect = %d Dest right(%d) left(%d) top(%d) bottom(%d) src_bitmap=
>          %p.\n", __FUNCTION__,
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index 2ce3ce5..77617b9 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -620,6 +620,7 @@ private:
>                              size_t alloc_size, PLIST_ENTRY pDelayed);
>      QXLDataChunk *MakeChunk(DelayedChunk *pdc);
>      ULONG PrepareDrawable(QXLDrawable*& drawable);
> +    void DiscardDrawable(QXLDrawable *drawable);
>      void AsyncIo(UCHAR  Port, UCHAR Value);
>      void SyncIo(UCHAR  Port, UCHAR Value);
>      NTSTATUS UpdateChildStatus(BOOLEAN connect);
> --
> 2.7.0.windows.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>