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.

Sign up for GitHub

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

Jump to bottom

Windows Installer does not broadcast a WM_SETTINGCHANGE #603

Closed
mathiask88 opened this issue Jan 25, 2015 · 11 comments
Closed

Windows Installer does not broadcast a WM_SETTINGCHANGE #603

mathiask88 opened this issue Jan 25, 2015 · 11 comments
Labels
windows Issues and PRs related to the Windows platform.

Comments

@mathiask88
Copy link
Contributor

Hi,

if I install io.js with the MSI the PATH changes do not take effect until restart/logout-login. This can be fixed if the MSI broadcasts a WM_SETTINGCHANGE message with lParam set to Environment. Anyone else with this problem? Maybe my windows is just broken :)
@piscisaureus
Copy link
Contributor

if I install io.js with the MSI the PATH changes do not take effect until restart/logout-login.

I have heard other people say the same but I've never been able to reproduce the issue.
For me it's always enough to open a new console window. I've tested this with various windows versions (XP, 7, 8.1).

This can be fixed if the MSI broadcasts a WM_SETTINGCHANGE message with lParam set to Environment.

I am not sure this is true. I don't think it has any effect on the scenario where the command window is re-opened after installation. Also, cmd.exe does not listen for WM_SETTINGCHANGE.

The installer source code is here: https://github.com/iojs/io.js/blob/v1.x/tools/msvs/msi/product.wxs
Feel free to poke around and see if you can fix the issue; improvements are welcome.

@mathiask88
Copy link
Contributor Author

I'm on Windows 8.1 x64 and if I run the cmd after installation as administrator the PATH is updated correct, but not in a non-administrator cmd (Maybe you have UAC disabled?) And I wrote a small console application that broadcasts the WM_SETTINGCHANGE message and then it was enough to open a new cmd as you said. Right the cmd.exe does not listen, but the explorer.exe does and is responsible for that behavior.

@piscisaureus
Copy link
Contributor

I'm on Windows 8.1 x64 and if I run the cmd after installation as administrator the PATH is updated correct, but not in a non-administrator cmd (Maybe you have UAC disabled?)

I'm using windows 8.1 too; whether I run elevated or non-elevated cmd.exe doesn't seem to matter here. (Slightly OT, I believe that in Windows 8.1 it's not really possible to turn UAC off. The only option you have is to not show the UAC popup).

And I wrote a small console application that broadcasts the WM_SETTINGCHANGE message and then it was enough to open a new cmd as you said. Right the cmd.exe does not listen, but the explorer.exe does and is responsible for that behavior.

I'll take your word for it. Is it possible to modify the installer so it broadcasts this message?

@Fishrock123 Fishrock123 added the windows Issues and PRs related to the Windows platform. label Jan 25, 2015
@piscisaureus
Copy link
Contributor

See also nodejs/node-v0.x-archive#2407

@mathiask88
Copy link
Contributor Author

Ok, if I reboot and run the MSI it works as expected, but if I uninstall and then install io.js again then the same behavior occurs. So in my case I removed node.js before I installed io.js and the node.js MSI set something like a "restart pending" flag and io.js MSI did not broadcast... But none of the installers say that a reboot IS necessary!

The usual restart pending registry keys that I know are HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Updates\UpdateExeVolatile and HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\PendingFileRenameOperations that not exist on my system.

I think if a reboot is not necessary then the MSI should not behave like this. But overall this is a rare case. Thanks @piscisaureus for your fast response!

@mathiask88
Copy link
Contributor Author

I referenced a fix. Now the installer always sends a WM_SETTINGCHANGE, even if you install and uninstall any number of times without rebooting/relogin. If you want to change the behavior I can create a PR.

@rvagg
Copy link
Member

rvagg commented Jan 26, 2015

Wow, that's crazy...

We could merge this and try it out in nightlies before committing to a release with this. If you don't get a quick -1 here @mathiask88 then you should PR this.

@mathiask88
Copy link
Contributor Author

@rvagg indeed, however it would be nice if someone could reproduce this.

@piscisaureus
Copy link
Contributor

@mathiask88 @rvagg

My suggestion would be to land that patch asap and ship it as part of a release to see if there is any fallout.
It's quite messy and doesn't conform to our code style, but I couldn't spot any bugs. I think it'd be good to see how well it fares in practice, and clean up later if it sticks.

@piscisaureus
Copy link
Contributor

... But none of the installers say that a reboot IS necessary!

That may be because we didn't define a dialog in which this shown.

@mathiask88
Copy link
Contributor Author

@piscisaureus Yes, this was a quick fix and I did not look into your code styles yet. I am not a professional , but I will do my best and try to match your styles and open a PR.

... But none of the installers say that a reboot IS necessary!

That may be because we didn't define a dialog in which this shown.

I ran the io.js MSI in verbose mode and the logfile did not contain any MsiSystemRebootPending property. So after all I really don't know why the WriteEnvironmentStrings only updates for the first setup. Even if I am not happy with this patch it fixes the issue.

@mathiask88 mathiask88 mentioned this issue Jan 26, 2015
Closed
piscisaureus pushed a commit to piscisaureus/node2 that referenced this issue Jan 27, 2015
@mathiask88 @piscisaureus
win,msi: broadcast WM_SETTINGCHANGE after install
668bde8 
In theory the msi should broadcast a 'WM_SETTINGCHANGE' message to all
windows after modifying the PATH environment variable. This ensures that
the new PATH is visible to other processes without restarting windows
(although it's still necessary to close and reopen active console
windows).

Unfortunately, the broadcast doesn't always happen, for unknown reasons.
That's why this patch adds a custom action that unconditionally
broadcasts a WM_SETTINGCHANGE message.

Bug: nodejs#603
PR: nodejs#613
Reviewed-by: Bert Belder <[email protected]>
@misterdjules misterdjules mentioned this issue Feb 18, 2015
Closed
@orangemocha orangemocha mentioned this issue May 6, 2015
Closed
orangemocha pushed a commit to nodejs/node-v0.x-archive that referenced this issue May 11, 2015
@mathiask88 @orangemocha
win,msi: broadcast WM_SETTINGCHANGE after install
e7c84f8 
Backport 668bde8 from io.js.
Original commit message follows:

In theory the msi should broadcast a 'WM_SETTINGCHANGE' message to all
windows after modifying the PATH environment variable. This ensures that
the new PATH is visible to other processes without restarting windows
(although it's still necessary to close and reopen active console
windows).

Unfortunately, the broadcast doesn't always happen, for unknown reasons.
That's why this patch adds a custom action that unconditionally
broadcasts a WM_SETTINGCHANGE message.

Bug: nodejs/node#603
PR: nodejs/node#613
Reviewed-by: Bert Belder <[email protected]>
(cherry picked from commit 668bde8)

--Node.js commmit metadata--
PR-URL: #25100
Reviewed-By: Julien Gilli <[email protected]>
Fixes: #4356
misterdjules pushed a commit to misterdjules/node that referenced this issue May 11, 2015
@mathiask88
win,msi: broadcast WM_SETTINGCHANGE after install
7935cfc 
Backport 668bde8 from io.js.
Original commit message follows:

In theory the msi should broadcast a 'WM_SETTINGCHANGE' message to all
windows after modifying the PATH environment variable. This ensures that
the new PATH is visible to other processes without restarting windows
(although it's still necessary to close and reopen active console
windows).

Unfortunately, the broadcast doesn't always happen, for unknown reasons.
That's why this patch adds a custom action that unconditionally
broadcasts a WM_SETTINGCHANGE message.

Bug: nodejs/node#603
PR: nodejs/node#613
Reviewed-by: Bert Belder <[email protected]>
(cherry picked from commit 668bde8)

--Node.js commmit metadata--
PR-URL: nodejs#25100
Reviewed-By: Julien Gilli <[email protected]>
Fixes: nodejs#4356
misterdjules pushed a commit to misterdjules/node that referenced this issue May 13, 2015
@mathiask88
win,msi: broadcast WM_SETTINGCHANGE after install
ed4b046 
Backport 668bde8 from io.js.
Original commit message follows:

In theory the msi should broadcast a 'WM_SETTINGCHANGE' message to all
windows after modifying the PATH environment variable. This ensures that
the new PATH is visible to other processes without restarting windows
(although it's still necessary to close and reopen active console
windows).

Unfortunately, the broadcast doesn't always happen, for unknown reasons.
That's why this patch adds a custom action that unconditionally
broadcasts a WM_SETTINGCHANGE message.

Bug: nodejs/node#603
PR: nodejs/node#613
Reviewed-by: Bert Belder <[email protected]>
(cherry picked from commit 668bde8)

--Node.js commmit metadata--
PR-URL: nodejs#25100
Reviewed-By: Julien Gilli <[email protected]>
Fixes: nodejs#4356
@maxdow maxdow mentioned this issue Sep 11, 2015
Closed
@mmashwani mmashwani mentioned this issue Oct 27, 2015
Closed
jBarz pushed a commit to ibmruntimes/node that referenced this issue Nov 4, 2016
@mathiask88 @jBarz
win,msi: broadcast WM_SETTINGCHANGE after install
7811c5e 
Backport 668bde8 from io.js.
Original commit message follows:

In theory the msi should broadcast a 'WM_SETTINGCHANGE' message to all
windows after modifying the PATH environment variable. This ensures that
the new PATH is visible to other processes without restarting windows
(although it's still necessary to close and reopen active console
windows).

Unfortunately, the broadcast doesn't always happen, for unknown reasons.
That's why this patch adds a custom action that unconditionally
broadcasts a WM_SETTINGCHANGE message.

Bug: nodejs/node#603
PR: nodejs/node#613
Reviewed-by: Bert Belder <[email protected]>
(cherry picked from commit 668bde8)

--Node.js commmit metadata--
PR-URL: nodejs#25100
Reviewed-By: Julien Gilli <[email protected]>
Fixes: nodejs#4356
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
windows Issues and PRs related to the Windows platform.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@piscisaureus @rvagg @Fishrock123 @mathiask88