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

Crashes explorer window sometimes #22

Open
masterofobzene opened this issue Feb 1, 2023 · 11 comments
Open

Crashes explorer window sometimes #22

masterofobzene opened this issue Feb 1, 2023 · 11 comments
Labels
bug Something isn't working Done question Further information is requested

Comments

@masterofobzene
Copy link

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
Copy link
Owner

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

@masterofobzene
Copy link
Author

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
Copy link
Owner

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

oleg-shilo added a commit that referenced this issue Feb 4, 2023
@masterofobzene
Copy link
Author

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

@oleg-shilo
Copy link
Owner

oleg-shilo commented Feb 4, 2023

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: dwmkerr/sharpshell#388

oleg-shilo added a commit that referenced this issue Feb 4, 2023
- Some improvements to minimize the frequency of #22
- Issue #20: Incorrect context menu
@masterofobzene
Copy link
Author

masterofobzene commented Feb 4, 2023

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
Copy link
Owner

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.

@oleg-shilo oleg-shilo added bug Something isn't working question Further information is requested Done - waiting for release labels Feb 5, 2023
@Ahernz
Copy link

Ahernz commented Apr 23, 2024

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
Copy link
Owner

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
Copy link

mstybay commented Jun 27, 2024

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
Copy link
Owner

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Done question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants