JFLarvoire / SysToolsLib

A library of Windows and Linux system management tools
Apache License 2.0
314 stars 93 forks source link

Onshutdown #33

Closed Daymarvi closed 2 weeks ago

Daymarvi commented 2 years ago

Hello,

I added some code to take into consideration the onshutdown, based on what i found on MS web https://docs.microsoft.com/en-us/dotnet/api/system.serviceprocess.servicebase.onshutdown?view=dotnet-plat-ext-6.0 the OnPreShutdown doens't exist but was necessary for us so i'm adding it there also. took the example from here : https://www.sivachandran.in/2012/03/handling-pre-shutdown-notification-in-c.html

JFLarvoire commented 2 years ago

Hello,

Sorry for the delay, I've been very busy these last few weeks. And, thanks, this is a very welcome addition to this PowerShell service! Before I pull the code, I have a few questions and comments:

One minor thing: You might want to append a short comment at the end of the header, describing your change. And while at it, change the date stamp in variable $scriptVersion.

One thing I don't understand: You've moved most of the C# method OnStop to a new method SCMStop(), shared between to OnStop() and OnPreshutdown(). This I understand, and this is fine. But what I don't understand is why OnStop() calls this.SCMStop(), then calls base.OnStop(). The latter base.OnStop() was not in the initial code. So what's the effect, and why is it needed?

As many people depend on this script, I want to be cautious, and avoid any risk of breaking anything. On what version of Windows did you test it?

I'm particulaly wary about the initialization code which throws an Exception("acceptedCommands field not found"); When would this possibly occur?

Finally, this gives me ideas for future improvements: (Here, I'm not requesting anything from you, beyond your opinion! (But if you want to do any one of these, go ahead! :-) ))