Skip to content

Commit

Permalink
Fix FolderNameEditor memory leak on cancel (#769)
Browse files Browse the repository at this point in the history
* Cleanup FolderNameEditor

* Match FolderBrowserDialog code in FolderNameEditor

Fix leaks

* PR feedback, more cleanup and fixes

* PR feedback
  • Loading branch information
hughbe authored and zsd4yr committed Apr 16, 2019
1 parent a9c3f60 commit 547a013
Show file tree
Hide file tree
Showing 24 changed files with 510 additions and 543 deletions.
11 changes: 11 additions & 0 deletions src/Common/src/Interop/Interop.HRESULT.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

internal static partial class Interop
{
public static class HRESULT
{
public const int S_OK = 0;
}
}
14 changes: 14 additions & 0 deletions src/Common/src/Interop/Interop.Libraries.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

internal static partial class Interop
{
public static partial class Libraries
{
internal const string Kernel32 = "kernel32.dll";
internal const string Gdi32 = "gdi32.dll";
internal const string User32 = "user32.dll";
internal const string Shell32 = "shell32.dll";
}
}
11 changes: 11 additions & 0 deletions src/Common/src/Interop/Kernel32/Interop.MAX_PATH.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

internal partial class Interop
{
internal partial class Kernel32
{
internal const int MAX_PATH = 260;
}
}
11 changes: 11 additions & 0 deletions src/Common/src/Interop/Kernel32/Interop.MAX_UNICODESTRING_LEN.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

internal partial class Interop
{
internal partial class Kernel32
{
internal const int MAX_UNICODESTRING_LEN = short.MaxValue;
}
}
53 changes: 53 additions & 0 deletions src/Common/src/Interop/Shell32/Interop.SHBrowseForFolder.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using Microsoft.Win32.SafeHandles;

internal static partial class Interop
{
public static partial class Shell32
{
[DllImport(Libraries.Shell32)]
public static extern CoTaskMemSafeHandle SHBrowseForFolderW(ref BROWSEINFO lpbi);

public delegate int BrowseCallbackProc(IntPtr hwnd, int msg, IntPtr lParam, IntPtr lpData);

public static class BrowseInfoFlags
{
public const uint BIF_RETURNONLYFSDIRS = 0x00000001;
public const uint BIF_DONTGOBELOWDOMAIN = 0x00000002;
public const uint BIF_RETURNFSANCESTORS = 0x00000008;
public const uint BIF_EDITBOX = 0x00000010;
public const uint BIF_NEWDIALOGSTYLE = 0x00000040;
public const uint BIF_NONEWFOLDERBUTTON = 0x00000200;

public const uint BIF_BROWSEFORCOMPUTER = 0x00001000;
public const uint BIF_BROWSEFORPRINTER = 0x00002000;
public const uint BIF_BROWSEFOREVERYTHING = 0x00004000;
}

[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Auto)]
public unsafe struct BROWSEINFO
{
public IntPtr hwndOwner;

public CoTaskMemSafeHandle pidlRoot;

public char *pszDisplayName;

public string lpszTitle;

public uint ulFlags;

public BrowseCallbackProc lpfn;

public IntPtr lParam;

public int iImage;
}
}
}
15 changes: 15 additions & 0 deletions src/Common/src/Interop/Shell32/Interop.SHGetKnownFolderPath.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Runtime.InteropServices;

internal static partial class Interop
{
public static partial class Shell32
{
[DllImport(Libraries.Shell32, CharSet = CharSet.Unicode, SetLastError = false, BestFitMapping = false, ExactSpelling = true)]
public static extern int SHGetKnownFolderPath(ref Guid rfid, uint dwFlags, IntPtr hToken, out string pszPath);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using Microsoft.Win32.SafeHandles;

internal static partial class Interop
{
public static partial class Shell32
{
[DllImport(ExternDll.Shell32, EntryPoint = "SHGetPathFromIDListEx", ExactSpelling = true)]
private static extern bool SHGetPathFromIDListEx(IntPtr pidl, IntPtr pszPath, int cchPath, int flags);

public static bool SHGetPathFromIDListLongPath(IntPtr pidl, out string path)
{
IntPtr pszPath = Marshal.AllocHGlobal((Interop.Kernel32.MAX_PATH + 1) * sizeof(char));
int length = Interop.Kernel32.MAX_PATH;
try
{
if (!SHGetPathFromIDListLongPath(pidl, ref pszPath, length))
{
path = null;
return false;
}

path = Marshal.PtrToStringAuto(pszPath);
return true;
}
finally
{
Marshal.FreeHGlobal(pszPath);
}
}

public unsafe static bool SHGetPathFromIDListLongPath(IntPtr pidl, ref IntPtr pszPath, int length)
{
// SHGetPathFromIDListEx is basically a helper to get IShellFolder.DisplayNameOf() with some
// extra functionally built in if the various flags are set.

// SHGetPathFromIDListEx copies into the ouput buffer using StringCchCopyW, which truncates
// when there isn't enough space (with a terminating null) and fails. Long paths can be
// extracted by simply increasing the buffer size whenever the buffer is full.

// To get the equivalent functionality we could call SHBindToParent on the PIDL to get IShellFolder
// and then invoke IShellFolder.DisplayNameOf directly. This would avoid long path contortions as
// we could directly convert from STRRET, calling CoTaskMemFree manually. (Presuming the type is
// STRRET_WSTR, of course. Otherwise we can just fall back to StrRetToBufW and give up for > MAX_PATH.
// Presumption is that we shouldn't be getting back ANSI results, and if we are they are likely
// some very old component that won't have a > MAX_PATH string.)

// While we could avoid contortions and avoid intermediate buffers by invoking IShellFolder directly,
// it isn't without cost as we'd be initializing a COM wrapper (RCW) for IShellFolder. Presumably
// this is much less overhead then looping and copying to intermediate buffers before creating a string.
// Additionally, implementing this would allow us to short circuit the one caller (FolderBrowserDialog)
// who doesn't care about the path, but just wants to know that we have an IShellFolder.
while (SHGetPathFromIDListEx(pidl, pszPath, length, 0) == false)
{
if (length >= Kernel32.MAX_UNICODESTRING_LEN
|| *(char*)pszPath.ToPointer() == '\0')
{
// Already at the maximum size string, or no data was copied in. Fail.
return false;
}

// Try giving the API a larger buffer
length = length * 2;
if (length > Kernel32.MAX_UNICODESTRING_LEN)
length = Kernel32.MAX_UNICODESTRING_LEN;

pszPath = Marshal.ReAllocHGlobal(pszPath, (IntPtr)(length * sizeof(char)));
}

return true;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Runtime.InteropServices;
using Microsoft.Win32.SafeHandles;

internal static partial class Interop
{
public static partial class Shell32
{
[DllImport(Libraries.Shell32, EntryPoint = "SHGetSpecialFolderLocation", ExactSpelling = true)]
public static extern int SHGetSpecialFolderLocation(IntPtr hwnd, int csidl, out CoTaskMemSafeHandle ppidl);
}
}
25 changes: 25 additions & 0 deletions src/Common/src/Microsoft/Win32/SafeHandles/CoTaskMemSafeHandle.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Runtime.InteropServices;

namespace Microsoft.Win32.SafeHandles
{
internal sealed class CoTaskMemSafeHandle : SafeHandle
{
internal CoTaskMemSafeHandle() : base(IntPtr.Zero, true)
{
}

public override bool IsInvalid => IsClosed || handle == IntPtr.Zero;

protected override bool ReleaseHandle()
{
Marshal.FreeCoTaskMem(handle);
handle = IntPtr.Zero;
return true;
}
}
}
28 changes: 3 additions & 25 deletions src/Common/src/NativeMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -361,27 +361,7 @@ public const int
CONNECT_E_NOCONNECTION = unchecked((int)0x80040200),
CONNECT_E_CANNOTCONNECT = unchecked((int)0x80040202),
CTRLINFO_EATS_RETURN = 1,
CTRLINFO_EATS_ESCAPE = 2,
CSIDL_DESKTOP = 0x0000, // <desktop>
CSIDL_INTERNET = 0x0001, // Internet Explorer (icon on desktop)
CSIDL_PROGRAMS = 0x0002, // Start Menu\Programs
CSIDL_PERSONAL = 0x0005, // My Documents
CSIDL_FAVORITES = 0x0006, // <user name>\Favorites
CSIDL_STARTUP = 0x0007, // Start Menu\Programs\Startup
CSIDL_RECENT = 0x0008, // <user name>\Recent
CSIDL_SENDTO = 0x0009, // <user name>\SendTo
CSIDL_STARTMENU = 0x000b, // <user name>\Start Menu
CSIDL_DESKTOPDIRECTORY = 0x0010, // <user name>\Desktop
CSIDL_TEMPLATES = 0x0015,
CSIDL_APPDATA = 0x001a, // <user name>\Application Data
CSIDL_LOCAL_APPDATA = 0x001c, // <user name>\Local Settings\Applicaiton Data (non roaming)
CSIDL_INTERNET_CACHE = 0x0020,
CSIDL_COOKIES = 0x0021,
CSIDL_HISTORY = 0x0022,
CSIDL_COMMON_APPDATA = 0x0023, // All Users\Application Data
CSIDL_SYSTEM = 0x0025, // GetSystemDirectory()
CSIDL_PROGRAM_FILES = 0x0026, // C:\Program Files
CSIDL_PROGRAM_FILES_COMMON = 0x002b; // C:\Program Files\Common
CTRLINFO_EATS_ESCAPE = 2;

public const int DUPLICATE = 0x06,
DISPID_UNKNOWN = (-1),
Expand Down Expand Up @@ -1007,8 +987,6 @@ public static int MAKELCID(int lgid, int sort) {


public const int MEMBERID_NIL = (-1),
MAX_PATH = 260,
MAX_UNICODESTRING_LEN = short.MaxValue, // maximum unicode string length
ERROR_INSUFFICIENT_BUFFER = 122, //https://msdn.microsoft.com/en-us/library/windows/desktop/ms681382(v=vs.85).aspx
MA_ACTIVATE = 0x0001,
MA_ACTIVATEANDEAT = 0x0002,
Expand Down Expand Up @@ -3270,9 +3248,9 @@ public class OPENFILENAME_I
public int nMaxCustFilter = 0;
public int nFilterIndex;
public IntPtr lpstrFile;
public int nMaxFile = NativeMethods.MAX_PATH;
public int nMaxFile = Interop.Kernel32.MAX_PATH;
public IntPtr lpstrFileTitle = IntPtr.Zero;
public int nMaxFileTitle = NativeMethods.MAX_PATH;
public int nMaxFileTitle = Interop.Kernel32.MAX_PATH;
public string lpstrInitialDir;
public string lpstrTitle;
public int Flags;
Expand Down
Loading

0 comments on commit 547a013

Please sign in to comment.