Re: [PATCH setup 0/2] List and offer to kill processes preventing a file from being written

[Jon TURNEY <jon dot turney at dronecode dot org dot uk>]

On 05/02/2013 17:51, Christopher Faylor wrote:
> On Tue, Feb 05, 2013 at 05:42:08PM +0000, Jon TURNEY wrote:
>> On 05/02/2013 16:16, Corinna Vinschen wrote:
>>> On Feb  5 11:06, Christopher Faylor wrote:
>>>> Wow.  Ambitious!
>>>>
>>>> On Tue, Feb 05, 2013 at 03:24:48PM +0000, Jon TURNEY wrote:
>>>>> I find it irritating to have to work out which process I need to stop when setup 
>>>>> can't update a file, and setup not helping you find it doesn't really meet 
>>>>> contemporary standards.  So, loosely inspired by [1], a patch to list and offer 
>>>>> to kill processes preventing a file from being written.
>>>>>
>>>>> This uses psapi.dll to find which out processes have a file loaded as a module.
>>>>>
>>>>> Note that this doesn't help when file isn't writeable because process has the 
>>>>> file open exclusively, but there doesn't seem to be an interface to do this 
>>>>> until the restart manager API introduced in Vista.
>>>>>
>>>>> This relies on the probably undocumented behaviour of /usr/bin/kill working with 
>>>>> windows PIDs as well as cygwin PIDs, and the assumption those PID sets are 
>>>>> disjoint.
>>>>
>>>> Why not just use TerminateProcess rather than Cygwin's /usr/bin/kill?
>>
>> Because sending SIGTERM gives the target process the opportunity to clean up?
> 
> Ok, but relying on kill.exe during installation seems a little iffy.  What if
> you've updated kill.exe but deferred on updating cygwin1.dll because there
> are executing processes using cygwin1.dll?  You could conceivably run into
> problems because kill.exe won't work with older dlls.  Admittedly it's not
> likely but it's definitely not foolproof.

I take your point, but I think we should try to terminate the processes
gracefully if we can.  If that fails and we end up having to TerminateProcess,
so be it.

I've re-written this slightly, so it tries kill -TERM, then kill -KILL, and
then TerminateProcess directly (rather than using kill -f), so this should
converge on the process being dead even if kill isn't available.

>>>> FWIW, Cygwin's pids often == Windows pids.  They are derived from the
>>>> same pool.
>>>>
>>>> I really like this idea but I wonder if the use of psapi.dll will mean
>>>> that setup-legacy.exe will be broken by that change since it is supposed
>>>> to work on older platforms.
>>
>> Yes, this probably doesn't work on older Windows versions (I have tested with
>> W2K and that is fine), although  information on API support in EOL'ed versions
>> of Windows is hard to find...
>>
>> I guess this could be changed to use the autoload mechanism for the functions
>> imported from psapi rather than linking with it.
>>
>>>> And, I wonder if it really is time to stop
>>>> offering the old 1.5.x binaries so we wouldn't have to worry about that.
>>>
>>> +1.
>>>
>>>> Other than that, I see some very minor formatting problems - you need to
>>>> put spaces before opening parentheses for functions and macros.

Fixed.

>>>>> Ideally, I wanted to note if the process which had the file loaded was a 
>>>>> service, and stop and restart the service.  But this seems to be not 
>>>>> straightforward to do, as setup doesn't have any visibility of the cygwin 
>>>>> process tree, which is needed to find the cygrunsrv service process which is the 
>>>>> parent of the process using the file, and thus the service name to stop and 
>>>>> restart.
>>>>
>>>> Is there any way to determine if a process is running as a service?  If
>>>> so, I'd think that just telling someone they had to restart the service
>>>> would be adequate.
>>
>> The problem is that (for example), sshd is not a service, it's a process
>> started by an instance of cygrunsrv which is registered with the sshd service
>> name and the sshd command line to run.
>>
>> I think that after finding that sshd is using some module we need to update,
>> setup would need to navigate the Cygwin process tree to find the cygrunsrv
>> process which is the parent of sshd to get the service name we would need to
>> restart.  I can't see how to do this in a purely windows application.
> 
> Playing both sides here: There are cygwin mechanisms for doing that in a
> pure windows program but if you're going to assume that kill exists why
> can't you assume that ps exists too?  You would, of course, have the same
> problem as I mentioned earlier though.
> 
> Doesn't psapi give you some mechanism for finding a pid's real parent?  I
> thought that was available in Windows.

Sorry, the detail I left out was that it appears that the cygwin PPID is not
the same as the windows PPID.  To continue the example, on my system, sshd has
cygrunsrv as it's cygwin parent process, but according to ProcessExplorer, it
has no parent process.

Certainly it would be possible to run some cygwin script to discover the
needed information (although it's not quite straightforward as ps doesn't seem
to accept Windows PIDs), I was just hoping for a more elegant way of doing things.