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.tur...@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.
Index: install.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/install.cc,v
retrieving revision 2.107
diff -u -u -r2.107 install.cc
--- install.cc 29 Jun 2013 20:48:58 -0000 2.107
+++ install.cc 23 Jul 2013 22:31:13 -0000
@@ -190,6 +190,44 @@
rebootneeded = true;
}
+#define MB_RETRYCONTINUE 7
+#if !defined(IDCONTINUE)
+#define IDCONTINUE IDCANCEL
+#endif
+
+static HHOOK hMsgBoxHook;
+LRESULT CALLBACK CBTProc(int nCode, WPARAM wParam, LPARAM lParam) {
+ HWND hWnd;
+ switch (nCode) {
+ case HCBT_ACTIVATE:
+ hWnd = (HWND)wParam;
+ if (GetDlgItem(hWnd, IDCANCEL) != NULL)
+ SetDlgItemText(hWnd, IDCANCEL, "Continue");
+ UnhookWindowsHookEx(hMsgBoxHook);
+ }
+ return CallNextHookEx(hMsgBoxHook, nCode, wParam, lParam);
+}
+
+int _custom_MessageBox(HWND hWnd, LPCTSTR szText, LPCTSTR szCaption, UINT
uType) {
+ int retval;
+ bool retry_continue = (uType & MB_TYPEMASK) == MB_RETRYCONTINUE;
+ if (retry_continue) {
+ uType &= ~MB_TYPEMASK; uType |= MB_RETRYCANCEL;
+ // Install a window hook, so we can intercept the message-box
+ // creation, and customize it
+ // Only install for THIS thread!!!
+ hMsgBoxHook = SetWindowsHookEx(WH_CBT, CBTProc, NULL,
GetCurrentThreadId());
+ }
+ retval = MessageBox(hWnd, szText, szCaption, uType);
+ // Intercept the return value for less confusing results
+ if (retry_continue && retval == IDCANCEL)
+ return IDCONTINUE;
+ return retval;
+}
+
+#undef MessageBox
+#define MessageBox _custom_MessageBox
+
typedef struct
{
const char *msg;
@@ -237,7 +275,7 @@
switch (LOWORD (wParam))
{
case IDRETRY:
- case IDOK:
+ case IDCONTINUE:
EndDialog (hwndDlg, LOWORD (wParam));
return TRUE;
}
@@ -475,40 +513,68 @@
plm += processName;
}
- INT_PTR rc = (iteration < 3) ? IDRETRY : IDOK;
+ INT_PTR rc = (iteration < 3) ? IDRETRY : IDCONTINUE;
if (unattended_mode == attended)
{
- FileInuseDlgData dlg_data;
- dlg_data.msg = ("Unable to extract /" + fn).c_str ();
- dlg_data.processlist = plm.c_str ();
- dlg_data.iteration = iteration;
+ if (!processes.empty())
+ {
+ // Use the IDD_FILE_INUSE dialog to ask the user
if we should try to kill the
+ // listed processes, or just ignore the problem
and schedule the file to be
+ // replaced after a reboot
+ FileInuseDlgData dlg_data;
+ dlg_data.msg = ("Unable to extract /" + fn).c_str
();
+ dlg_data.processlist = plm.c_str ();
+ dlg_data.iteration = iteration;
- rc = DialogBoxParam(hinstance, MAKEINTRESOURCE
(IDD_FILE_INUSE), owner, FileInuseDlgProc, (LPARAM)&dlg_data);
+ rc = DialogBoxParam(hinstance, MAKEINTRESOURCE
(IDD_FILE_INUSE), owner, FileInuseDlgProc, (LPARAM)&dlg_data);
+ }
+ else
+ {
+ // We couldn't enumerate any processes which have
this file loaded into it
+ // either the cause of the error is something
else, or something (e.g security
+ // policy) prevents us from listing those
processes.
+ // All we can offer the user is a generic "retry
or ignore" choice and a chance
+ // to fix the problem themselves
+ char msg[fn.size() + 300];
+ sprintf (msg,
+ "Unable to extract /%s\r\n"
+ "The file is in use or some other error
occurred.\r\n"
+ "Please stop all Cygwin processes and
select \"Retry\", or\r\n"
+ "select \"Continue\" to go on anyway (you
will need to reboot).\r\n",
+ fn.c_str());
+
+ rc = MessageBox (owner, msg, "Error writing file",
+ MB_RETRYCONTINUE | MB_ICONWARNING
| MB_TASKMODAL);
+ }
}
switch (rc)
{
case IDRETRY:
- // try to stop all the processes
- for (ProcessList::iterator i = processes.begin (); i
!= processes.end (); i++)
- {
- i->kill (iteration);
- }
-
- // wait up to 15 seconds for processes to stop
- for (unsigned int i = 0; i < 15; i++)
+ if (!processes.empty())
{
- processes = Process::listProcessesWithModuleLoaded
(sname);
- if (processes.size () == 0)
- break;
+ // try to stop all the processes
+ for (ProcessList::iterator i = processes.begin ();
i != processes.end (); i++)
+ {
+ i->kill (iteration);
+ }
+
+ // wait up to 15 seconds for processes to stop
+ for (unsigned int i = 0; i < 15; i++)
+ {
+ processes =
Process::listProcessesWithModuleLoaded (sname);
+ if (processes.size () == 0)
+ break;
- Sleep (1000);
+ Sleep (1000);
+ }
}
+ // else, manual intervention may have fixed the problem
// retry
iteration++;
continue;
- case IDOK:
+ case IDCONTINUE:
// ignore this in-use error, and any subsequent in-use
errors for other files in the same package
ignoreInUseErrors = true;
break;
Index: res.rc
===================================================================
RCS file: /cvs/cygwin-apps/setup/res.rc,v
retrieving revision 2.100
diff -u -u -r2.100 res.rc
--- res.rc 22 Jun 2013 20:02:01 -0000 2.100
+++ res.rc 23 Jul 2013 22:31:13 -0000
@@ -457,7 +457,7 @@
"select 'Continue' to go on anyway (you will need to
reboot).",
IDC_FILE_INUSE_HELP,27,52,183,16,NOT WS_GROUP
DEFPUSHBUTTON "&Stop Processes",IDRETRY,47,75,55,15
- PUSHBUTTON "&Continue",IDOK,113,75,55,15
+ PUSHBUTTON "&Continue",IDCONTINUE,113,75,55,15
END
/////////////////////////////////////////////////////////////////////////////
