This is the mail archive of the
cygwin-apps
mailing list for the Cygwin project.
Re: [PATCH setup 2/2] List and offer to kill processes preventing a file from being written
- From: Christopher Faylor <cgf-use-the-mailinglist-please at cygwin dot com>
- To: cygwin-apps at cygwin dot com
- Date: Wed, 24 Jul 2013 01:23:03 -0400
- Subject: Re: [PATCH setup 2/2] List and offer to kill processes preventing a file from being written
- References: <1360077890-3508-3-git-send-email-jon dot turney at dronecode dot org dot uk> <1360180983-1836-1-git-send-email-jon dot turney at dronecode dot org dot uk> <20130207050651 dot GA2898 at ednor dot casa dot cgf dot cx> <51EF0696 dot 8000807 at dronecode dot org dot uk>
- Reply-to: cygwin-apps 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