dahall / Vanara

A set of .NET libraries for Windows implementing PInvoke calls to many native Windows APIs with supporting wrappers.
MIT License
1.75k stars 190 forks source link

ShellItemImages.GetImageAsync might throw NullReferenceException #414

Closed zhuxb711 closed 11 months ago

zhuxb711 commented 1 year ago

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.

https://github.com/dahall/Vanara/blob/f66686c287b2a69e6fbbd5336ccbf3f4296b97a0/Windows.Shell.Common/ShellObjects/ShellItemImages.cs#L50-L64

https://github.com/dahall/Vanara/blob/f66686c287b2a69e6fbbd5336ccbf3f4296b97a0/Windows.Shell.Common/ShellObjects/ShellItem.cs#L502-L509

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()

https://github.com/dahall/Vanara/blob/f66686c287b2a69e6fbbd5336ccbf3f4296b97a0/Windows.Shell.Common/ShellObjects/ShellItem.cs#L699

https://github.com/dahall/Vanara/blob/f66686c287b2a69e6fbbd5336ccbf3f4296b97a0/Windows.Shell.Common/ShellObjects/ShellItemImages.cs#L30

dahall commented 1 year ago

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 commented 1 year ago

@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 commented 11 months ago

Done