oleg-shilo / shell-x

Dynamic context menu manager for Windows.
144 stars 10 forks source link

Crashes explorer window sometimes #22

Open masterofobzene opened 1 year ago

masterofobzene commented 1 year ago

It crashes the explorer window that I'm using the contextual menu on, closing the window and reopening it fixes the problem. The problem is random. Sometimes the shell-x menu dissapears. Sometimes the explorer window crashes and black or white rectangles start appearing on the window menues.

The most common is the contextual menu completely dissappearing and leaving a grey rectangle only: 2023-02-01 13-10-34

Env: Windows 10 x64 | 90+ custom menu entries each with icons

oleg-shilo commented 1 year ago

Thank you @masterofobzene Any idea how it can be reproduced?

masterofobzene commented 1 year ago

Thank you @masterofobzene Any idea how it can be reproduced?

I suppose you can try adding more than 90 "dumb" entries and adding an icon to them, then start right clicking on an explorer window to spawn the menu at least 10 times, try to do it more since it happens suddenly (it works normally at the beginning). I'm using this large menu in the [any] folder since I need to use the menu for many filetypes and since the menu is so big, I don't want to make copies of it in each [extension folder]

oleg-shilo commented 1 year ago

Good news. I found the cause. The menu is not disposed after it's been used and it leads to memory exhaustion due to the many loaded menu icons. I do not know if it is a flaw in the explorer or in the SharpShell.

Will see if I can programmatically dispose the icons if I can detect the closing even though it's not the responsibility of the shell extension :(

masterofobzene commented 1 year ago

great it was a memory leak then? hope it can be fixed

oleg-shilo commented 1 year ago

The good news is that I know execly what causing the problem - the icons.

The bad news is that it cannot be fixed because it is not a fault of the extension or SharpShell.dll but of Windows Explorer.

My speculation was that icons were not disposed of due to the garbage collector not being called frequently enough and the host process (explorer + SharpShell.dll) not disposing of the menu explicitly on deactivation.

Indeed it is the case SharpShell.dll never disposes ContextMenuStrip. But it is because it uses the strip only as a menu blueprint to create a native menu from it and then passes it to the explorer process. Thus disposing was left to the GC.

After looking at SharpShell.dll code I found that it does not keep any copy of toolstrip or its icons so it cannot possibly have abandoned instances of the menu(s). Thus it is not causing the leaks.

So I implemented explicit disposing of the old menu strip when the old one is created. But it only gives a little less pressure on the memory. That's it. The explorer process still accumulates native menus with the images and (in my case) kills the memory when ~250-300 menu items with the image are popped (25 times menus with 100 items with images).

The "smoking gun" evidence came after I altered the code to ever create a single instance of that 100 items menu and always return the same instance every time user right-clicks the file. It still kills the explorer after 25 cycles.

Thus the only way to avoid the problem is to minimize or even drop the use of icons :(

I will log the issue on SharpShell project so maybe the author can have some workaround. But the chance is slim.

[UPD] Done. Logged the issue: https://github.com/dwmkerr/sharpshell/issues/388

masterofobzene commented 1 year ago

Wow, nice investigation. Thanks for your time, I would help if I had the expertise. I'm going to drop the icons and see if it works better here.

Can this be of any help? https://stackoverflow.com/questions/32572238/how-do-i-fix-a-memory-leak-cause-by-contextmenustrips-not-disposing

oleg-shilo commented 1 year ago

Unfortunately no, that post is not relevant to the problem. It is about mem-leaks in the WinForm application caused by multiple ContextMenuStrip not being disposed by the user.

In our case, it is the explorer who does not do it though not for ContextMenuStrip but its native equivalent. And the reason for my confidence is that "smoking gun" evidence. In that experiment, I ensured that the ContextMenuStrip instance is created only once (not possible in real-life situations) and yet the explorer was still leaking with the same rate.

If you drop icons, your scenario most likely will work just fine.

I will also need on, home page, to warn users about impact of heavy use of icons.

Ahernz commented 5 months ago

I have an idea. If I set the icon as the default icon for Windows, for example by directly reading the icon from the “SHELL32.dll” file, then it should take up less memory. Can this problem be solved?

oleg-shilo commented 5 months ago

Unlikely, as it will still create an icon. Though not from your custom dll but SHELL32.dll.

Saying that Windows might be smart enough to detect that you are trying to open the icon that is a stock icon and if there is a cache then reuse it.

I say, try your idea. It's a simple test. I would give it a 30% chance to succeed. But if it does, the outcome is excellent. SHELL32.dl arguably has all icons that you ever need.

mstybay commented 2 months ago

I experience the same problem that explorer.exe crashing due to icon overlay. Did you find anything for it? And do Dave still working on it?

oleg-shilo commented 2 months ago

Hi Mustafa, @Ahernz proposed the idea and I suggested he/she test it. This is where I left it. You see, shell-x creates the shell extension and uses it the way it's intended. So nothing there to be fixed. And the unfortunate behaviour is a result of some resource mismanagement in the explorer. @Ahernz's idea was just about creating the circumstances for the explorer to use different resources and possibly avoid the problem.

Unfortunately, I do not see any good workaround. Of course, you can also drop the use of icons altogether.

Who is Dave?