Skip to content
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

ShellItemImages.GetImageAsync might throw NullReferenceException #414

Closed
zhuxb711 opened this issue Jul 6, 2023 · 3 comments
Closed

ShellItemImages.GetImageAsync might throw NullReferenceException #414

zhuxb711 opened this issue Jul 6, 2023 · 3 comments

Comments

@zhuxb711
Copy link

zhuxb711 commented Jul 6, 2023

Describe the bug and how to reproduce

Recently, I got some stacktrace and found that ShellItemImages.GetImageAsync might throw NRE.

image

image

Although the stacktrace do not show any useful information, but I think this issue might be related to the ShellItem.Parent. ShellItem.Parent might return null.

if (hr != HRESULT.S_OK && !flags.IsFlagSet(ShellItemGetImageOptions.IconOnly))
hr = LoadImageFromExtractImage(shellItem.Parent.IShellFolder, shellItem.PIDL.LastId, ref sz, out hbmp);
if (hr != HRESULT.S_OK)
{
if (!flags.IsFlagSet(ShellItemGetImageOptions.ThumbnailOnly))
{
LoadIconFromExtractIcon(shellItem.Parent.IShellFolder, shellItem.PIDL.LastId, ref sz, out SafeHICON hIcon).ThrowIfFailed();
using (hIcon)
hbmp = hIcon.ToHBITMAP();
}
else
{
throw hr.GetException();
}
}

public ShellFolder Parent
{
get
{
try { return new ShellFolder(iShellItem.GetParent()); } catch { }
return null;
}
}

What code is involved

using (ShellItem Item = new ShellItem("<Your path>"))
using (Gdi32.SafeHBITMAP HBitmap = Item.GetImage(new SIZE(150, 150), ShellItemGetImageOptions.BiggerSizeOk))
{

}

Expected behavior
Should not throw NullReferenceException

By the way, it would be better to call the sync function directly rather than wait for Task.Result here, you simply warp the function in Task.Run()

public SafeHBITMAP GetImage(SIZE size, ShellItemGetImageOptions flags) => Images.GetImageAsync(size, flags).Result;

public async Task<SafeHBITMAP> GetImageAsync(SIZE size, ShellItemGetImageOptions flags = 0, bool forcePreVista = false) => await TaskAgg.Run(() =>

dahall added a commit that referenced this issue Jul 7, 2023
@dahall
Copy link
Owner

dahall commented Jul 7, 2023

I applied what I think is a fix for the NRE -- many more null checks. On your second point about wrapping the function in Task.Run, I don't see your request. Can you show how you'd do it for those two method calls?

@zhuxb711
Copy link
Author

@dahall What I mean is that GetImage should not use Task internally. Let the caller to decide whether to warp it in the Task.Run

public SafeHBITMAP GetImage(SIZE size, ShellItemGetImageOptions flags) => Images.GetImage(size, flags); 

dahall added a commit that referenced this issue Jul 17, 2023
@dahall
Copy link
Owner

dahall commented Aug 7, 2023

Done

@dahall dahall closed this as completed Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants