-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Fix memory leak in IIS StartupHook by properly disposing PhysicalFileProvider #63322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…Provider Co-authored-by: danmoseley <[email protected]>
@BrennanConroy any opinion on whether ErrorPageModelBuilder.CreateErrorPageModel() ought to do the dispose here? Indeed it is not used after that call so the fix as is is reasonable. |
The |
OK. Are you comfortable with the dispose in this particular context? ie this change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a potential memory leak in the IIS StartupHook class by ensuring proper disposal of a PhysicalFileProvider
instance used in unhandled exception handling.
- Wrapped
PhysicalFileProvider
creation in ausing
statement to ensure automatic disposal - Replaced inline instantiation with a properly scoped variable reference
- Applied existing codebase patterns for resource management with file providers
@@ -49,8 +49,10 @@ public static void Initialize() | |||
var iisConfigData = NativeMethods.HttpGetApplicationProperties(); | |||
var contentRoot = iisConfigData.pwzFullApplicationPath.TrimEnd(Path.DirectorySeparatorChar); | |||
|
|||
using var fileProvider = new PhysicalFileProvider(contentRoot); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a blank line after the using statement declaration to improve readability and follow the formatting guidelines that specify inserting newlines before code blocks.
Copilot uses AI. Check for mistakes.
The SVACE static analyzer identified a potential handle leak in the IIS StartupHook class where a
PhysicalFileProvider
was created but not properly disposed.Problem
In
src/Servers/IIS/IIS/src/StartupHook.cs
, the unhandled exception handler creates aPhysicalFileProvider
to generate error page content but doesn't dispose it:Since
PhysicalFileProvider
implementsIDisposable
and manages file system resources, this could lead to handle leaks.Solution
Wrapped the
PhysicalFileProvider
creation in ausing
statement to ensure proper disposal:This approach follows existing patterns in the codebase where
PhysicalFileProvider
is used withusing
statements in similar scenarios (e.g., in diagnostic and static file middleware tests).The fix is safe because
ErrorPageModelBuilder.CreateErrorPageModel()
only uses the file provider during its execution and doesn't retain any references to it.Fixes #59524.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.