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

Different behavior of the same mod from repo and installed locally. #171

Closed
Anixx opened this issue Mar 5, 2024 · 7 comments
Closed

Different behavior of the same mod from repo and installed locally. #171

Anixx opened this issue Mar 5, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@Anixx
Copy link

Anixx commented Mar 5, 2024

The mod Classic Explorer Treeview when installed locally works just well, but when installed from repo it is not activated upon Windhawk startup and needs switching off and on to start working. The code of the mod is the same in both cases.

This issue have been experieced by other users as well.

@Anixx Anixx added the bug Something isn't working label Mar 5, 2024
@m417z
Copy link
Member

m417z commented Mar 5, 2024

That's unlikely, Windhawk compiles and loads the mod in the same way in both cases. Please try to come up with simple steps to reproduce the problem and I'll check it out.

@m417z m417z added the info-needed Further information is requested label Mar 5, 2024
@CyprinusCarpio
Copy link

CyprinusCarpio commented Mar 5, 2024

I can reproduce this problem. The mod when installed from the repo, and when it's being enabled at explorer.exe startup, fails to retrieve a handle (GetModuleHandleW) to explorerframe.dll in Wh_ModInit, and returns false. But when the mod is enabled after explorer is started, it works just fine.

Makes no sense given the previous comment, but it appears as if the mod from the repo gets initialized before the explorerframe.dll is loaded in explorer.exe process.

Regarding Classic Explorer Treeview, a workaround may be to use LoadLibraryW in case GetModuleHandleW fails to produce a valid handle. I'll add that to my mod, and make a pull request, along with a one simplification I was working on regarding the expando buttons.

@CyprinusCarpio
Copy link

Not sure on the complexity of such a solution, but maybe it's possible to add a new mod option that will dictate what modules need to be loaded in the process before the mod is allowed to initialize, such as
@require explorerframe.dll

@m417z
Copy link
Member

m417z commented Mar 5, 2024

when it's being enabled at explorer.exe startup [...]
[...]
But when the mod is enabled after explorer is started [...]

So it seems that the problem isn't about a repo vs. local mod, it's about when the mod is enabled.

a workaround may be to use LoadLibraryW in case GetModuleHandleW fails to produce a valid handle

Makes sense, I even wouldn't call it a workaround, it seems like a reasonable fix to me. And I see no reason to call GetModuleHandleW in this case, just use LoadLibraryW right away.

maybe it's possible to add a new mod option [...] such as
@require explorerframe.dll

What advantages would that have over just using LoadLibraryW? If anything, it's more limiting, as you might need to do some actions before calling LoadLibraryW.

@CyprinusCarpio
Copy link

CyprinusCarpio commented Mar 5, 2024

For that last part, I meant adding a new possible entry to the ==WindhawkMod== mod header that will require a specified module to be loaded in the target process before the mod is allowed to initialize. So that GetModuleHandleW for that specified module is guaranteed to return a valid handle.

But I'm not sure about the practicality of such a approach. If I understand correctly, Windhawk gets hold of a process as soon as possible, and my proposal potentially contradicts that, because it could warrant a wait for the specified module to be loaded in the process, which we know will happen eventually with explorer.exe and explorerframe.dll, but this can't be guaranteed for other combinations.

Anyways, I'll make use of the more practical approach which is to use LoadLibraryW. I'm fairly positive that explorerframe.dll doesn't normally get unloaded as long as explorer.exe is running, so I think it's ok to disregard the ref count.

But none of this actually addresses the issue as reported, matter of fact is that to the end user my mod (as it is currently) works when it's being used as a local fork, but it doesn't when it's being used from the repo. I'm partly at fault for naively assuming that explorerframe.dll will have been loaded in the process by the time my mod initializes. At least 4 people, me included, have reproduced this problem.
One person is saying:

I have similar issue. If I change some mod settings, it does work for some time, then stops again. I use version from Windhawk repo.

After the mod initialization fails, and the mod is left "enabled", will changing the mod settings provoke a another mod initialization attempt?

@m417z
Copy link
Member

m417z commented Mar 6, 2024

For that last part, I meant adding a new possible entry to the ==WindhawkMod== mod header that will require a specified module to be loaded in the target process before the mod is allowed to initialize. So that GetModuleHandleW for that specified module is guaranteed to return a valid handle.

So what you're saying is: only load the mod when a specified dll is loaded by the process? For now, you can implement something like this yourself by hooking LoadLibraryW or equivalent. I did something similar here. But in your case, I think just calling LoadLibrary for explorerframe.dll yourself is fine.

we know will happen eventually with explorer.exe and explorerframe.dll

BTW I'm not sure about that, e.g. if a explorer.exe process doesn't show any open folder (maybe folders are opened in separate processes). But in any case, I don't think loading explorerframe.dll, even if redundant, would affect performance in a noticible way.

I'm fairly positive that explorerframe.dll doesn't normally get unloaded as long as explorer.exe is running, so I think it's ok to disregard the ref count.

I agree.

to the end user my mod (as it is currently) works when it's being used as a local fork, but it doesn't when it's being used from the repo

I replied to this in my first comment:

That's unlikely, Windhawk compiles and loads the mod in the same way in both cases. Please try to come up with simple steps to reproduce the problem and I'll check it out.

That might also be related to the mod loading order. The order of loaded mods is unspecified, and is likely to be the lexicographical order of the mod ids due to the way registry enumeration works. Perhaps the local mod loads at a later time and so there's a higher chance that explorerframe.dll is already loaded. If that's the explanation, then there's nothing to fix and it works as intended.

After the mod initialization fails, and the mod is left "enabled", will changing the mod settings provoke a another mod initialization attempt?

Yes. You can see a bit more details here.

@m417z m417z removed the info-needed Further information is requested label Mar 6, 2024
@m417z
Copy link
Member

m417z commented Mar 6, 2024

I think that we've established that there's no issue here. If you have a reason to think otherwise, let me know and I'll reopen the issue.

@m417z m417z closed this as completed Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants