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

[Christopher Faylor <cgf-use-the-mailinglist-please at cygwin dot com>]

On Tue, Jul 23, 2013 at 11:41:26PM +0100, Jon TURNEY wrote:
>On 07/02/2013 05:06, Christopher Faylor wrote:
>> On Wed, Feb 06, 2013 at 08:03:03PM +0000, Jon TURNEY wrote:
>>> - Enumerate processes preventing a file from being written
>>> - Replace the MessageBox reporting an in-use file with a DialogBox reporting the
>>> in-use file and the processes which are using that file
>>> - Offer to kill processes which have files open, trying /usr/bin/kill with SIGTERM,
>>> then SIGKILL, then TerminateProcess
>>>
>>> v2:
>>> - Use TerminateProcess directly, rather than using kill -f, so we will work even
>>> if kill doesn't
>>> - Fix formatting: spaces before parentheses for functions and macros
>> 
>> Thanks for doing this.  I know that the setup source code doesn't follow this
>> rule consistently.
>> 
>> Please check in.  This looks like a really nice change.
>
>Unfortunately, this change seems to have a few problems (See [1]):  If we
>can't enumerate the processes which are preventing a file from being written,
>or some other error occurs which archive::extract_file() doesn't know how to
>report (e.g. disk full), the user is presented with the dialog with an empty
>process list, and it's unclear how they should proceed (Ideally finding the
>processes and stopping them themselves, as before...)
>
>(In testing, on my W7 x86_64 system, it seems that service processes using a
>file we want to replace could not be listed, even by an Adminstrator account.
> I'm not sure if it's possible to do anything about that or not...)
>
>I couldn't really come up with a good way to show this in the new
>IDD_FILE_INUSE dialog, so attached is patch which restores the custom message
>box which was used previously to ask 'retry or continue' , and uses that when
>the process list is empty.
>
>If in future archive::extract_file() learns how to report other conditions,
>there's perhaps some re-factoring which can go on here so IDD_FILE_INUSE is
>used when we know the error was a sharing violation, and this 'retry or
>continue' custom message box can be used to handle generic errors.
>
>[1] http://cygwin.com/ml/cygwin/2013-07/msg00483.html
>
>2013-07-23  Jon TURNEY  <jon.turney@dronecode.org.uk>
>
>	* install.cc (_custom_MessageBox): Restore custom message box.
>	(installOne): If processList is empty, use the custom message box
>	to ask if we should retry or continue.
>	* res.rc (IDD_FILE_INUSE): Use IDCONTINUE for continue button, to be
>	the same custom message box.

This is exactly what I was thinking we should do when I discussed this
with you on IRC.  Thank you.

Please check this in and I'll generate a new setup.exe ASAP.

Btw, it's nice to be able to kill processes that have opened files.  I
was delighted to see this dialog crop up during an installation.  It
saved me the effort of having to find and kill the process.

cgf