Merge pull request #678 from Icinga:fix/core_memory_leaks

LordHepipud · web-flow · commit 5305f7e14c1f · 2024-02-02T10:49:11.000+01:00
Fix: Various memory leak fixes and improvements
diff --git a/.github/ISSUE_TEMPLATE/create-new-issue.md b/.github/ISSUE_TEMPLATE/create-new-issue.md
diff --git a/.gitignore b/.gitignore
@@ -8,6 +8,7 @@ cache/*
 *.log
 *.pfx
 *.dll
+*.DS_Store
 
 # JEA
 RoleCapabilities/IcingaForWindows.psrc
diff --git a/doc/100-General/10-Changelog.md b/doc/100-General/10-Changelog.md
@@ -18,6 +18,7 @@ Released closed milestones can be found on [GitHub](https://github.com/Icinga/ic
 ### Bugfixes
 
 * [#673](https://github.com/Icinga/icinga-powershell-framework/pull/673) Fixes a memory leak while fetching Windows EventLog information by using CLI tools and inside the Hyper-V provide
+* [#678](https://github.com/Icinga/icinga-powershell-framework/pull/678) Fixes various memory leaks in Icinga for Windows API core and check handler
 
 ## 1.11.1 (2023-11-07)
 
diff --git a/lib/core/cache/Get-IcingaCacheData.psm1 b/lib/core/cache/Get-IcingaCacheData.psm1
@@ -33,7 +33,8 @@ function Get-IcingaCacheData()
         [string]$Space,
         [string]$CacheStore,
         [string]$KeyName,
-        [switch]$TempFile   = $FALSE
+        [switch]$TempFile   = $FALSE,
+        [switch]$AsObject   = $FALSE
     );
 
     $CacheFile       = Join-Path -Path (Join-Path -Path (Join-Path -Path (Get-IcingaCacheDir) -ChildPath $Space) -ChildPath $CacheStore) -ChildPath ([string]::Format('{0}.json', $KeyName));
@@ -62,7 +63,7 @@ function Get-IcingaCacheData()
         return $null;
     }
 
-    if ([string]::IsNullOrEmpty($KeyName)) {
+    if ($AsObject -Or [string]::IsNullOrEmpty($KeyName)) {
         return $cacheData;
     } else {
         return $cacheData.$KeyName;
diff --git a/lib/core/cache/Set-IcingaCacheData.psm1 b/lib/core/cache/Set-IcingaCacheData.psm1
@@ -43,7 +43,7 @@ function Set-IcingaCacheData()
     }
 
     if ((Test-Path $CacheFile)) {
-        $cacheData = Get-IcingaCacheData -Space $Space -CacheStore $CacheStore;
+        $cacheData = Get-IcingaCacheData -Space $Space -CacheStore $CacheStore -KeyName $KeyName -AsObject;
     } else {
         try {
             New-Item -ItemType File -Path $CacheTmpFile -Force -ErrorAction Stop | Out-Null;
diff --git a/lib/core/logging/Write-IcingaDebugMessage.psm1 b/lib/core/logging/Write-IcingaDebugMessage.psm1
@@ -18,4 +18,6 @@ function Write-IcingaDebugMessage()
     $DebugContent += $Objects;
 
     Write-IcingaEventMessage -EventId 1000 -Namespace 'Debug' -ExceptionObject $ExceptionObject -Objects $DebugContent;
+
+    $DebugContent = $null;
 }
diff --git a/lib/daemons/RestAPI/client/Test-IcingaRESTClientConnection.psm1 b/lib/daemons/RestAPI/client/Test-IcingaRESTClientConnection.psm1
@@ -11,7 +11,7 @@ function Test-IcingaRESTClientConnection()
             -Client $Connection.Client `
             -ClientList $Global:Icinga.Public.Daemons.RESTApi.ClientBlacklist;
         Write-IcingaEventMessage -EventId 1501 -Namespace 'Framework' -Objects $Connection.Client.Client;
-        Close-IcingaTCPConnection -Client $Connection.Client;
+        Close-IcingaTCPConnection -Connection $Connection;
         $Connection = $null;
         return $FALSE;
     }
diff --git a/lib/daemons/RestAPI/daemon/New-IcingaForWindowsRESTApi.psm1 b/lib/daemons/RestAPI/daemon/New-IcingaForWindowsRESTApi.psm1
@@ -84,9 +84,15 @@ function New-IcingaForWindowsRESTApi()
             -Client (New-IcingaTCPClient -Socket $Socket) `
             -Certificate $Certificate;
 
+        if ($Connection.Client -eq $null -Or $Connection.Stream -eq $null) {
+            Close-IcingaTCPConnection -Connection $Connection;
+            $Connection = $null;
+            continue;
+        }
+
         if (Test-IcingaRESTClientBlacklisted -Client $Connection.Client -ClientList $Global:Icinga.Public.Daemons.RESTApi.ClientBlacklist) {
             Write-IcingaDebugMessage -Message 'A remote client which is trying to connect was blacklisted' -Objects $Connection.Client.Client;
-            Close-IcingaTCPConnection -Client $Connection.Client;
+            Close-IcingaTCPConnection -Connection $Connection;
             $Connection = $null;
             continue;
         }
@@ -98,7 +104,7 @@ function New-IcingaForWindowsRESTApi()
 
         # API not yet ready
         if ($Global:Icinga.Public.Daemons.RESTApi.ApiRequests.Count -eq 0) {
-            Close-IcingaTCPConnection -Client $Connection.Client;
+            Close-IcingaTCPConnection -Connection $Connection;
             $Connection = $null;
             continue;
         }
@@ -107,7 +113,7 @@ function New-IcingaForWindowsRESTApi()
             $NextRESTApiThreadId = (Get-IcingaNextRESTApiThreadId);
 
             if ($Global:Icinga.Public.Daemons.RESTApi.ApiRequests.ContainsKey($NextRESTApiThreadId) -eq $FALSE) {
-                Close-IcingaTCPConnection -Client $Connection.Client;
+                Close-IcingaTCPConnection -Connection $Connection;
                 $Connection = $null;
                 continue;
             }
diff --git a/lib/daemons/RestAPI/threads/New-IcingaForWindowsRESTThread.psm1 b/lib/daemons/RestAPI/threads/New-IcingaForWindowsRESTThread.psm1
@@ -38,7 +38,7 @@ function New-IcingaForWindowsRESTThread()
                         # Send the authentication prompt
                         Send-IcingaWebAuthMessage -Connection $Connection;
                         # Close the connection
-                        Close-IcingaTCPConnection -Client $Connection.Client;
+                        Close-IcingaTCPConnection -Connection $Connection;
                         $Connection = $null;
                         continue;
                     }
@@ -56,14 +56,14 @@ function New-IcingaForWindowsRESTThread()
                         # Re-send the authentication prompt
                         Send-IcingaWebAuthMessage -Connection $Connection;
                         # Close the connection
-                        Close-IcingaTCPConnection -Client $Connection.Client;
+                        Close-IcingaTCPConnection -Connection $Connection;
                         $Connection = $null;
                         continue;
                     }
                 }
 
                 # Set our thread being active
-                Set-IcingaForWindowsThreadAlive -ThreadName $Global:Icinga.Protected.ThreadName -Active -TerminateAction @{ 'Command' = 'Close-IcingaTCPConnection'; 'Arguments' = @{ 'Client' = $Connection.Client } };
+                Set-IcingaForWindowsThreadAlive -ThreadName $Global:Icinga.Protected.ThreadName -Active -TerminateAction @{ 'Command' = 'Close-IcingaTCPConnection'; 'Arguments' = @{ 'Connection' = $Connection } };
 
                 # We should remove clients from the blacklist who are sending valid requests
                 Remove-IcingaRESTClientBlacklist -Client $Connection.Client -ClientList $Global:Icinga.Public.Daemons.RESTApi.ClientBlacklist;
@@ -100,7 +100,7 @@ function New-IcingaForWindowsRESTThread()
 
         # Finally close the clients connection as we are done here and
         # ensure this thread will close by simply leaving the function
-        Close-IcingaTCPConnection -Client $Connection.Client;
+        Close-IcingaTCPConnection -Connection $Connection;
         $Connection = $null;
 
         # Force Icinga for Windows Garbage Collection
diff --git a/lib/icinga/plugin/New-IcingaCheckResult.psm1 b/lib/icinga/plugin/New-IcingaCheckResult.psm1
@@ -40,11 +40,17 @@ function New-IcingaCheckResult()
 
         Set-IcingaInternalPluginExitCode -ExitCode $ExitCode;
 
+        $this.Check = $null;
+
         return $ExitCode;
     }
 
     if ($Compile) {
-        return $IcingaCheckResult.Compile();
+        $IcingaExitCode    = $IcingaCheckResult.Compile();
+        $IcingaCheckResult = $null;
+        $Check             = $null;
+
+        return $IcingaExitCode;
     }
 
     return $IcingaCheckResult;
diff --git a/lib/webserver/Close-IcingaTCPConnection.psm1 b/lib/webserver/Close-IcingaTCPConnection.psm1
@@ -1,21 +1,40 @@
 function Close-IcingaTCPConnection()
 {
     param(
-        [System.Net.Sockets.TcpClient]$Client = $null
+        [hashtable]$Connection = @{ }
     );
 
-    if ($null -eq $Client) {
+    if ($null -eq $Connection -Or $Connection.Count -eq 0) {
+        $Connection = $null;
+
         return;
     }
 
     Write-IcingaDebugMessage -Message (
         [string]::Format(
             'Closing client connection for endpoint {0}',
-            (Get-IcingaTCPClientRemoteEndpoint -Client $Client)
+            (Get-IcingaTCPClientRemoteEndpoint -Client $Connection.Client)
         )
     );
 
-    $Client.Close();
-    $Client.Dispose();
-    $Client = $null;
+    try {
+        if ($null -ne $Connection.Stream) {
+            $Connection.Stream.Close();
+            $Connection.Stream.Dispose();
+            $Connection.Stream = $null;
+        }
+    } catch {
+        $Connection.Stream = $null;
+    }
+    try {
+        if ($null -ne $Connection.Client) {
+            $Connection.Client.Close();
+            $Connection.Client.Dispose();
+            $Connection.Client = $null;
+        }
+    } catch {
+        $Connection.Client = $null;
+    }
+
+    $Connection = $null;
 }
diff --git a/lib/webserver/New-IcingaSSLStream.psm1 b/lib/webserver/New-IcingaSSLStream.psm1
@@ -9,10 +9,17 @@ function New-IcingaSSLStream()
         return $null;
     }
 
+    [System.Net.Security.SslStream]$SSLStream = $null;
+
     try {
-        $SSLStream = New-Object System.Net.Security.SslStream($Client.GetStream(), $false)
+        $SSLStream = New-Object System.Net.Security.SslStream($Client.GetStream(), $false);
         $SSLStream.AuthenticateAsServer($Certificate, $false, [System.Security.Authentication.SslProtocols]::Tls12, $true) | Out-Null;
     } catch {
+        if ($null -ne $SSLStream) {
+            $SSLStream.Close();
+            $SSLStream.Dispose();
+            $SSLStream = $null;
+        }
         Write-IcingaEventMessage -EventId 1500 -Namespace 'Framework' -ExceptionObject $_ -Objects $Client.Client;
         return $null;
     }

Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ function Set-IcingaCacheData()`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`if ((Test-Path $CacheFile)) {`
`46`		`- $cacheData = Get-IcingaCacheData -Space $Space -CacheStore $CacheStore;`
	`46`	`+ $cacheData = Get-IcingaCacheData -Space $Space -CacheStore $CacheStore -KeyName $KeyName -AsObject;`
`47`	`47`	`} else {`
`48`	`48`	`try {`
`49`	`49`	`New-Item -ItemType File -Path $CacheTmpFile -Force -ErrorAction Stop \| Out-Null;`
Original file line number	Diff line number	Diff line change
`@@ -18,4 +18,6 @@ function Write-IcingaDebugMessage()`
`18`	`18`	`$DebugContent += $Objects;`
`19`	`19`
`20`	`20`	`Write-IcingaEventMessage -EventId 1000 -Namespace 'Debug' -ExceptionObject $ExceptionObject -Objects $DebugContent;`
	`21`	`+`
	`22`	`+ $DebugContent = $null;`
`21`	`23`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ function Test-IcingaRESTClientConnection()`
`11`	`11`	-Client $Connection.Client `
`12`	`12`	`-ClientList $Global:Icinga.Public.Daemons.RESTApi.ClientBlacklist;`
`13`	`13`	`Write-IcingaEventMessage -EventId 1501 -Namespace 'Framework' -Objects $Connection.Client.Client;`
`14`		`- Close-IcingaTCPConnection -Client $Connection.Client;`
	`14`	`+ Close-IcingaTCPConnection -Connection $Connection;`
`15`	`15`	`$Connection = $null;`
`16`	`16`	`return $FALSE;`
`17`	`17`	`}`