* [PATCH setup 1/2] Refactor ::run() so it's more generally useful
2013-02-05 15:26 [PATCH setup 0/2] List and offer to kill processes preventing a file from being written Jon TURNEY
2013-02-05 15:26 ` [PATCH setup 2/2] " Jon TURNEY
@ 2013-02-05 15:26 ` Jon TURNEY
2013-02-05 16:07 ` [PATCH setup 0/2] List and offer to kill processes preventing a file from being written Christopher Faylor
2 siblings, 0 replies; 12+ messages in thread
From: Jon TURNEY @ 2013-02-05 15:26 UTC (permalink / raw)
To: cygwin-apps; +Cc: Jon TURNEY
Move all the logging of the command it runs in
Move the formatting of the command line used for postinstall script running out
2013-02-01 Jon TURNEY <jon.turney@dronecode.org.uk>
* script.cc (::run, Script::run): Move the formatting of the command
line used for postinstall script running out to Script::run. Move the
logging of the command and it's output into ::run.
* script.h: Add ::run() prototype.
Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
script.cc | 33 +++++++++++++++++++--------------
script.h | 7 +++++--
2 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/script.cc b/script.cc
index 6663e8c..4bf0f09 100644
--- a/script.cc
+++ b/script.cc
@@ -196,10 +196,10 @@ OutputLog::out_to(std::ostream &out)
SetFilePointer(_handle, 0, NULL, FILE_END);
}
-static int
-run (const char *sh, const char *args, const char *file, OutputLog &file_out)
+int
+run (const char *cmdline)
{
- char cmdline[MAX_PATH];
+
STARTUPINFO si;
PROCESS_INFORMATION pi;
DWORD flags = CREATE_NEW_CONSOLE;
@@ -207,7 +207,11 @@ run (const char *sh, const char *args, const char *file, OutputLog &file_out)
BOOL inheritHandles = FALSE;
BOOL exitCodeValid = FALSE;
- sprintf (cmdline, "%s %s \"%s\"", sh, args, file);
+ log(LOG_PLAIN) << "running: " << cmdline << endLog;
+
+ char tmp_pat[] = "/var/log/setup.log.runXXXXXXX";
+ OutputLog file_out = std::string (mktemp (tmp_pat));
+
memset (&pi, 0, sizeof (pi));
memset (&si, 0, sizeof (si));
si.cb = sizeof (si);
@@ -226,7 +230,7 @@ run (const char *sh, const char *args, const char *file, OutputLog &file_out)
flags = CREATE_NO_WINDOW; // Note: this is ignored on Win9x
}
- BOOL createSucceeded = CreateProcess (0, cmdline, 0, 0, inheritHandles,
+ BOOL createSucceeded = CreateProcess (0, (char *)cmdline, 0, 0, inheritHandles,
flags, 0, get_root_dir ().c_str(),
&si, &pi);
@@ -237,6 +241,10 @@ run (const char *sh, const char *args, const char *file, OutputLog &file_out)
}
CloseHandle(pi.hProcess);
CloseHandle(pi.hThread);
+
+ if (!file_out.isEmpty ())
+ log(LOG_BABBLE) << file_out << endLog;
+
if (exitCodeValid)
return exitCode;
return -GetLastError();
@@ -268,24 +276,21 @@ Script::run() const
}
int retval;
- char tmp_pat[] = "/var/log/setup.log.postinstallXXXXXXX";
- OutputLog file_out = std::string (mktemp (tmp_pat));
+ char cmdline[MAX_PATH];
+
if (sh.size() && stricmp (extension(), ".sh") == 0)
{
- log(LOG_PLAIN) << "running: " << sh << " --norc --noprofile \"" << scriptName << "\"" << endLog;
- retval = ::run (sh.c_str(), "--norc --noprofile", scriptName.c_str(), file_out);
+ sprintf (cmdline, "%s %s \"%s\"", sh.c_str(), "--norc --noprofile", scriptName.c_str());
+ retval = ::run (cmdline);
}
else if (cmd && stricmp (extension(), ".bat") == 0)
{
- log(LOG_PLAIN) << "running: " << cmd << " /c \"" << windowsName << "\"" << endLog;
- retval = ::run (cmd, "/c", windowsName.c_str(), file_out);
+ sprintf (cmdline, "%s %s \"%s\"", cmd, "/c", windowsName.c_str());
+ retval = ::run (cmdline);
}
else
return -ERROR_INVALID_DATA;
- if (!file_out.isEmpty ())
- log(LOG_BABBLE) << file_out << endLog;
-
if (retval)
log(LOG_PLAIN) << "abnormal exit: exit code=" << retval << endLog;
diff --git a/script.h b/script.h
index 144fd71..abdd43e 100644
--- a/script.h
+++ b/script.h
@@ -14,7 +14,7 @@
*/
#ifndef SETUP_SCRIPT_H
#define SETUP_SCRIPT_H
-
+
/* Initialisation stuff for run_script: sh, cmd, CYGWINROOT and PATH */
void init_run_script ();
@@ -24,6 +24,9 @@ int try_run_script (const std::string& dir,
const std::string& fname,
const std::string& ext);
+/* Run a command and capture it's output to the log */
+int run (const char *cmdline);
+
class Script {
public:
static bool isAScript (const std::string& file);
@@ -32,7 +35,7 @@ public:
std::string fullName() const;
/* Run the script. If its suffix is .sh, and we have a Bourne shell, execute
it using sh. Otherwise, if the suffix is .bat, execute using cmd.exe (NT)
- or command.com (9x). Returns the exit status of the process, or
+ or command.com (9x). Returns the exit status of the process, or
negative error if any. */
int run() const;
bool operator == (const Script s) { return s.scriptName == scriptName; } ;
--
1.7.9
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH setup 2/2] List and offer to kill processes preventing a file from being written
2013-02-05 15:26 [PATCH setup 0/2] List and offer to kill processes preventing a file from being written Jon TURNEY
@ 2013-02-05 15:26 ` Jon TURNEY
2013-02-06 20:03 ` Jon TURNEY
2013-02-05 15:26 ` [PATCH setup 1/2] Refactor ::run() so it's more generally useful Jon TURNEY
2013-02-05 16:07 ` [PATCH setup 0/2] List and offer to kill processes preventing a file from being written Christopher Faylor
2 siblings, 1 reply; 12+ messages in thread
From: Jon TURNEY @ 2013-02-05 15:26 UTC (permalink / raw)
To: cygwin-apps; +Cc: Jon TURNEY
- 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.
- Use /usr/bin/kill to kill processes which have files open, trying SIGTERM,
then SIGKILL, then TerminateProcess
2013-02-01 Jon TURNEY <jon.turney@dronecode.org.uk>
* install.cc ( _custom_MessageBox): Remove custom message box.
(FileInuseDlgProc): Add file-in-use dialog box.
(installOne): Use processlist to list processes using a file, and
offer to kill them with the file-in-use dialog.
* res.rc (IDD_FILE_INUSE) : New dialog.
* resource.h (IDD_FILE_INUSE, IDC_FILE_INUSE_EDIT)
(IDC_FILE_INUSE_MSG, IDC_FILE_INUSE_HELP): Define corresponding
resource ID numbers.
* processlist.h: New file.
* processlist.cc: New file.
* Makefile.am (setup_LDADD): Add -lpsapi.
(setup_SOURCES): Add new files.
Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
Makefile.am | 4 +-
install.cc | 152 +++++++++++++++++++++++++-----------
processlist.cc | 237 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
processlist.h | 41 ++++++++++
res.rc | 20 +++++
resource.h | 4 +
6 files changed, 411 insertions(+), 47 deletions(-)
create mode 100644 processlist.cc
create mode 100644 processlist.h
diff --git a/Makefile.am b/Makefile.am
index 0f1498b..ddd19ed 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -105,7 +105,7 @@ inilint_SOURCES = \
setup_LDADD = \
libgetopt++/libgetopt++.la -lgcrypt -lgpg-error \
- -lshlwapi -lcomctl32 -lole32 -lwsock32 -lnetapi32 -luuid -llzma -lbz2 -lz
+ -lshlwapi -lcomctl32 -lole32 -lwsock32 -lnetapi32 -lpsapi -luuid -llzma -lbz2 -lz
setup_LDFLAGS = -mwindows -Wc,-static -static-libtool-libs
setup_SOURCES = \
AntiVirus.cc \
@@ -230,6 +230,8 @@ setup_SOURCES = \
postinstallresults.h \
prereq.cc \
prereq.h \
+ processlist.cc \
+ processlist.h \
proppage.cc \
proppage.h \
propsheet.cc \
diff --git a/install.cc b/install.cc
index 9d39f33..61333b7 100644
--- a/install.cc
+++ b/install.cc
@@ -63,6 +63,7 @@ static const char *cvsid = "\n%%% $Id: install.cc,v 2.101 2011/07/25 14:36:24 jt
#include "threebar.h"
#include "Exception.h"
+#include "processlist.h"
using namespace std;
@@ -192,39 +193,61 @@ Installer::replaceOnRebootSucceeded (const std::string& fn, bool &rebootneeded)
rebootneeded = true;
}
-#define MB_RETRYCONTINUE 7
-#if !defined(IDCONTINUE)
-#define IDCONTINUE IDCANCEL
-#endif
+typedef struct
+{
+ const char *msg;
+ const char *processlist;
+ int iteration;
+} FileInuseDlgData;
-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);
-}
+static BOOL CALLBACK
+FileInuseDlgProc(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam)
+{
+ switch (uMsg)
+ {
+ case WM_INITDIALOG:
+ {
+ FileInuseDlgData *dlg_data = (FileInuseDlgData *)lParam;
+
+ SetDlgItemText(hwndDlg, IDC_FILE_INUSE_MSG, dlg_data->msg);
+ SetDlgItemText(hwndDlg, IDC_FILE_INUSE_EDIT, dlg_data->processlist);
+
+ switch (dlg_data->iteration)
+ {
+ case 0:
+ break; // show the dialog the way it is in the resource
+
+ case 1:
+ SetDlgItemText(hwndDlg, IDRETRY, "&Kill Processes");
+ SetDlgItemText(hwndDlg, IDC_FILE_INUSE_HELP,
+ "Select 'Kill' to kill Cygwin processes and retry, or "
+ "select 'Continue' to go on anyway (you will need to reboot).");
+ break;
+
+ default:
+ case 2:
+ SetDlgItemText(hwndDlg, IDRETRY, "&Kill Processes");
+ SetDlgItemText(hwndDlg, IDC_FILE_INUSE_HELP,
+ "Select 'Kill' to forcibly kill all processes and retry, or "
+ "select 'Continue' to go on anyway (you will need to reboot).");
+ }
+ }
+ return TRUE; // automatically set focus, please
-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;
+ case WM_COMMAND:
+ if (HIWORD(wParam) == BN_CLICKED)
+ {
+ switch (LOWORD(wParam))
+ {
+ case IDRETRY:
+ case IDOK:
+ EndDialog(hwndDlg, LOWORD (wParam));
+ return TRUE;
+ }
+ }
+ }
+
+ return FALSE;
}
/* Helper function to create the registry value "AllowProtectedRenames",
@@ -316,9 +339,6 @@ Installer::extract_replace_on_reboot (archive *tarstream, const std::string& pre
return false;
}
-#undef MessageBox
-#define MessageBox _custom_MessageBox
-
static char all_null[512];
/* install one source at a given prefix. */
@@ -427,7 +447,7 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
}
bool error_in_this_package = false;
- bool ignoreInUseErrors = unattended_mode;
+ bool ignoreInUseErrors = false;
bool ignoreExtractErrors = unattended_mode;
package_bytes = source.size;
@@ -448,7 +468,7 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
if (Script::isAScript (fn))
pkgm.desired.addScript (Script (canonicalfn));
- bool firstIteration = true;
+ int iteration = 0;
int extract_error = 0;
while ((extract_error = archive::extract_file (tarstream, prefixURL, prefixPath)) != 0)
{
@@ -458,21 +478,61 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
{
if (!ignoreInUseErrors)
{
- char msg[fn.size() + 300];
- sprintf (msg,
- "%snable to extract /%s -- the file is in use.\r\n"
- "Please stop %s Cygwin processes and select \"Retry\", or\r\n"
- "select \"Continue\" to go on anyway (you will need to reboot).\r\n",
- firstIteration?"U":"Still u", fn.c_str(), firstIteration?"all":"ALL");
+ // convert the file name to long UNC form
+ std::string s = backslash(cygpath("/" + fn));
+ WCHAR sname[s.size () + 7];
+ mklongpath(sname, s.c_str (), s.size () + 7);
+
+ // find any process which has that file loaded into it
+ // (note that this doesn't find when the file is un-writeable because the process has
+ // that file opened exclusively)
+ ProcessList processes = Process::listProcessesWithModuleLoaded(sname);
+
+ std::string plm;
+ for (ProcessList::iterator i = processes.begin(); i != processes.end(); i++)
+ {
+ if (i != processes.begin()) plm += "\r\n";
- switch (MessageBox (owner, msg, "In-use files detected",
- MB_RETRYCONTINUE | MB_ICONWARNING | MB_TASKMODAL))
+ std::string processName = i->getName();
+ log (LOG_BABBLE) << processName << endLog;
+ plm += processName;
+ }
+
+ INT_PTR rc = (iteration < 3) ? IDRETRY : IDOK;
+ 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;
+
+ rc = DialogBoxParam(hinstance, MAKEINTRESOURCE(IDD_FILE_INUSE), owner, FileInuseDlgProc, (LPARAM)&dlg_data);
+ }
+
+ 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++)
+ {
+ processes = Process::listProcessesWithModuleLoaded(sname);
+ if (processes.size() == 0)
+ break;
+
+ Sleep(1000);
+ }
+
// retry
- firstIteration = false;
+ iteration++;
continue;
- case IDCONTINUE:
+ case IDOK:
+ // ignore this in-use error, and any subsequent in-use errors for other files in the same package
ignoreInUseErrors = true;
break;
default:
diff --git a/processlist.cc b/processlist.cc
new file mode 100644
index 0000000..2e925e3
--- /dev/null
+++ b/processlist.cc
@@ -0,0 +1,237 @@
+/*
+ * Copyright (c) 2013 Jon TURNEY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * A copy of the GNU General Public License can be found at
+ * http://www.gnu.org/
+ *
+ */
+
+#include <windows.h>
+#define PSAPI_VERSION 1
+#include <psapi.h>
+#include <stdio.h>
+
+#include "processlist.h"
+#include <String++.h>
+#include "LogSingleton.h"
+#include "script.h"
+#include "mount.h"
+#include "filemanip.h"
+
+// ---------------------------------------------------------------------------
+// implements class Process
+//
+// access to a Windows process
+// ---------------------------------------------------------------------------
+
+Process::Process(DWORD pid) : processID(pid)
+{
+}
+
+std::string
+Process::getName(void)
+{
+ HANDLE hProcess = OpenProcess( PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, processID);
+ char modName[MAX_PATH];
+ GetModuleFileNameExA(hProcess, NULL, modName, sizeof(modName));
+ CloseHandle(hProcess);
+
+ std::string s = modName;
+ s += " (pid " + stringify(processID) + ")";
+ return s;
+}
+
+DWORD
+Process::getProcessID(void)
+{
+ return processID;
+}
+
+void
+Process::kill(int force)
+{
+ std::string kill_cmd = backslash(cygpath("/bin/kill.exe")) + " ";
+
+ switch (force)
+ {
+ case 0:
+ kill_cmd += "-TERM";
+ break;
+
+ case 1:
+ kill_cmd += "-KILL";
+ break;
+
+ default:
+ case 2:
+ // -f means 'use TerminateProcess() if the kill(2) function fails'
+ // really we just want to TerminateProcess() here
+ kill_cmd += "-KILL -f";
+ break;
+ }
+
+ kill_cmd += " " + stringify(processID);
+ ::run(kill_cmd.c_str());
+}
+
+//
+// test if a module is loaded into a process
+//
+bool
+Process::isModuleLoadedInProcess(const WCHAR *moduleName)
+{
+ BOOL match = FALSE;
+
+ // Get process handle
+ HANDLE hProcess = OpenProcess( PROCESS_QUERY_INFORMATION |
+ PROCESS_VM_READ,
+ FALSE, processID );
+
+ if (NULL == hProcess)
+ return FALSE;
+
+ static unsigned int bytesAllocated = 0;
+ static HMODULE *hMods = 0;
+
+ // initial allocation
+ if (bytesAllocated == 0)
+ {
+ bytesAllocated = sizeof(HMODULE)*1024;
+ hMods = (HMODULE *)malloc(bytesAllocated);
+ }
+
+ while (1)
+ {
+ DWORD cbNeeded;
+
+ // Get a list of all the modules in this process.
+ if (!EnumProcessModules(hProcess, hMods, bytesAllocated, &cbNeeded))
+ {
+ // Don't log ERROR_PARTIAL_COPY as expected for System process and those of different bitness
+ if (GetLastError() != ERROR_PARTIAL_COPY)
+ {
+ log (LOG_BABBLE) << "EnumProcessModules failed " << GetLastError() << " for pid " << processID << endLog;
+ }
+
+ cbNeeded = 0;
+ }
+
+ // If we didn't get all modules, retry with a larger array
+ if (cbNeeded > bytesAllocated)
+ {
+ bytesAllocated = cbNeeded;
+ hMods = (HMODULE *)realloc(hMods, bytesAllocated);
+ continue;
+ }
+
+ // Search module list for the module we are looking for
+ for (int i = 0; i < (cbNeeded / sizeof(HMODULE)); i++ )
+ {
+ WCHAR szModName[MAX_PATH];
+
+ // Get the full path to the module's file.
+ if (GetModuleFileNameExW(hProcess, hMods[i], szModName, sizeof(szModName)/sizeof(WCHAR)))
+ {
+ WCHAR canonicalModName[MAX_PATH];
+
+ // Canonicalise returned module name to long UNC form
+ if (wcscmp(szModName, L"\\\\?\\") != 0)
+ {
+ wcscpy(canonicalModName, L"\\\\?\\");
+ wcscat(canonicalModName, szModName);
+ }
+ else
+ {
+ wcscpy(canonicalModName, szModName);
+ }
+
+ // Does it match the name ?
+ if (wcscmp(moduleName, canonicalModName) == 0)
+ {
+ match = TRUE;
+ break;
+ }
+ }
+ }
+
+ break;
+ }
+
+ // Release the process handle
+ CloseHandle(hProcess);
+
+ return match;
+}
+
+//
+// get a list of currently running processes
+//
+ProcessList
+Process::snapshot(void)
+{
+ static DWORD *pProcessIDs = 0;
+ static unsigned int bytesAllocated = 0;
+ DWORD bytesReturned;
+
+ // initial allocation
+ if (bytesAllocated == 0)
+ {
+ bytesAllocated = sizeof(DWORD);
+ pProcessIDs = (DWORD *)malloc(bytesAllocated);
+ }
+
+ // fetch a snapshot of process list
+ while (1)
+ {
+ if (!EnumProcesses(pProcessIDs, bytesAllocated, &bytesReturned))
+ {
+ log (LOG_BABBLE) << "EnumProcesses failed " << GetLastError() << endLog;
+ bytesReturned = 0;
+ }
+
+ // If we didn't get all processes, retry with a larger array
+ if (bytesReturned == bytesAllocated)
+ {
+ bytesAllocated = bytesAllocated*2;
+ pProcessIDs = (DWORD *)realloc(pProcessIDs, bytesAllocated);
+ continue;
+ }
+
+ break;
+ }
+
+ // convert to ProcessList vector
+ unsigned int nProcesses = bytesReturned/sizeof(DWORD);
+ ProcessList v(nProcesses, 0);
+ for (unsigned int i = 0; i < nProcesses; i++)
+ {
+ v[i] = pProcessIDs[i];
+ }
+
+ return v;
+}
+
+//
+// list processes which have a given executable module loaded
+//
+ProcessList
+Process::listProcessesWithModuleLoaded(const WCHAR *moduleName)
+{
+ ProcessList v;
+ ProcessList pl = snapshot();
+
+ for (ProcessList::iterator i = pl.begin(); i != pl.end(); i++)
+ {
+ if (i->isModuleLoadedInProcess(moduleName))
+ {
+ v.push_back(*i);
+ }
+ }
+
+ return v;
+}
diff --git a/processlist.h b/processlist.h
new file mode 100644
index 0000000..db03a3d
--- /dev/null
+++ b/processlist.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2013 Jon TURNEY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * A copy of the GNU General Public License can be found at
+ * http://www.gnu.org/
+ *
+ */
+
+#include <windows.h>
+#include <vector>
+#include <string>
+
+// ---------------------------------------------------------------------------
+// interface to class Process
+//
+// access to a Windows process
+// ---------------------------------------------------------------------------
+
+class Process;
+
+// utility type ProcessList, a vector of Process
+typedef std::vector<Process> ProcessList;
+
+class Process
+{
+public:
+ Process(DWORD pid);
+ std::string getName(void);
+ DWORD getProcessID(void);
+ void kill(int force);
+ bool isModuleLoadedInProcess(const WCHAR *moduleName);
+ static ProcessList listProcessesWithModuleLoaded(const WCHAR *moduleName);
+private:
+ DWORD processID;
+ static ProcessList snapshot(void);
+};
diff --git a/res.rc b/res.rc
index d6c0ff0..4bc8de1 100644
--- a/res.rc
+++ b/res.rc
@@ -440,6 +440,26 @@ BEGIN
END
+IDD_FILE_INUSE DIALOG DISCARDABLE 0, 0, SETUP_SMALL_DIALOG_DIMS
+STYLE DS_MODALFRAME | DS_CENTER | WS_POPUP | WS_CAPTION
+CAPTION "In-use file detected"
+FONT 8, "MS Shell Dlg"
+BEGIN
+ ICON IDI_WARNING,IDC_HEADICON,5,5
+ LTEXT "Unable to extract %s",
+ IDC_FILE_INUSE_MSG,27,5,183,8,SS_PATHELLIPSIS
+ LTEXT "The file is in use by the following processes:",
+ IDC_STATIC,27,14,183,8
+ EDITTEXT IDC_FILE_INUSE_EDIT,27,23,183,28,WS_VSCROLL |
+ ES_LEFT | ES_MULTILINE | ES_READONLY |
+ ES_AUTOVSCROLL | NOT WS_TABSTOP
+ LTEXT "Select 'Stop' to stop Cygwin processes and retry, or "
+ "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
+END
+
/////////////////////////////////////////////////////////////////////////////
//
// Manifest
diff --git a/resource.h b/resource.h
index df68473..99a6f42 100644
--- a/resource.h
+++ b/resource.h
@@ -64,6 +64,7 @@
#define IDD_PREREQ 220
#define IDD_DROPPED 221
#define IDD_POSTINSTALL 222
+#define IDD_FILE_INUSE 223
// Bitmaps
@@ -173,3 +174,6 @@
#define IDC_CHOOSE_CLEAR_SEARCH 587
#define IDC_LOCAL_DIR_DESC 588
#define IDC_POSTINSTALL_EDIT 589
+#define IDC_FILE_INUSE_EDIT 590
+#define IDC_FILE_INUSE_MSG 591
+#define IDC_FILE_INUSE_HELP 592
--
1.7.9
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH setup 0/2] List and offer to kill processes preventing a file from being written
@ 2013-02-05 15:26 Jon TURNEY
2013-02-05 15:26 ` [PATCH setup 2/2] " Jon TURNEY
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Jon TURNEY @ 2013-02-05 15:26 UTC (permalink / raw)
To: cygwin-apps; +Cc: Jon TURNEY
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.
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.
[1] http://cygwin.com/ml/cygwin/2012-08/msg00444.html
Jon TURNEY (2):
Refactor ::run() so it's more generally useful
List and offer to kill processes preventing a file from being written
Makefile.am | 4 +-
install.cc | 152 +++++++++++++++++++++++++-----------
processlist.cc | 237 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
processlist.h | 41 ++++++++++
res.rc | 20 +++++
resource.h | 4 +
script.cc | 33 +++++----
script.h | 7 +-
8 files changed, 435 insertions(+), 63 deletions(-)
create mode 100644 processlist.cc
create mode 100644 processlist.h
--
1.7.9
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH setup 0/2] List and offer to kill processes preventing a file from being written
2013-02-05 15:26 [PATCH setup 0/2] List and offer to kill processes preventing a file from being written Jon TURNEY
2013-02-05 15:26 ` [PATCH setup 2/2] " Jon TURNEY
2013-02-05 15:26 ` [PATCH setup 1/2] Refactor ::run() so it's more generally useful Jon TURNEY
@ 2013-02-05 16:07 ` Christopher Faylor
2013-02-05 16:16 ` Corinna Vinschen
2 siblings, 1 reply; 12+ messages in thread
From: Christopher Faylor @ 2013-02-05 16:07 UTC (permalink / raw)
To: cygwin-apps
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?
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. 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.
Other than that, I see some very minor formatting problems - you need to
put spaces before opening parentheses for functions and macros.
>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.
cgf
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH setup 0/2] List and offer to kill processes preventing a file from being written
2013-02-05 16:07 ` [PATCH setup 0/2] List and offer to kill processes preventing a file from being written Christopher Faylor
@ 2013-02-05 16:16 ` Corinna Vinschen
2013-02-05 17:42 ` Jon TURNEY
0 siblings, 1 reply; 12+ messages in thread
From: Corinna Vinschen @ 2013-02-05 16:16 UTC (permalink / raw)
To: cygwin-apps
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?
> 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. 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.
>
> >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.
Starting with Vista you know a process is running as service if it's
running in session 0. Other than that, if the process is called
cygrunsrv, or if the process parent is called cygrunsrv, it's pretty
likely that it's a service.
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH setup 0/2] List and offer to kill processes preventing a file from being written
2013-02-05 16:16 ` Corinna Vinschen
@ 2013-02-05 17:42 ` Jon TURNEY
2013-02-05 17:51 ` Christopher Faylor
0 siblings, 1 reply; 12+ messages in thread
From: Jon TURNEY @ 2013-02-05 17:42 UTC (permalink / raw)
To: cygwin-apps
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?
>> 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.
>>
>>> 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.
> Starting with Vista you know a process is running as service if it's
> running in session 0. Other than that, if the process is called
> cygrunsrv, or if the process parent is called cygrunsrv, it's pretty
> likely that it's a service.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH setup 0/2] List and offer to kill processes preventing a file from being written
2013-02-05 17:42 ` Jon TURNEY
@ 2013-02-05 17:51 ` Christopher Faylor
2013-02-06 20:07 ` Jon TURNEY
0 siblings, 1 reply; 12+ messages in thread
From: Christopher Faylor @ 2013-02-05 17:51 UTC (permalink / raw)
To: cygwin-apps
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.
>>> 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.
>>>
>>>> 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.
cgf
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH setup 2/2] List and offer to kill processes preventing a file from being written
2013-02-05 15:26 ` [PATCH setup 2/2] " Jon TURNEY
@ 2013-02-06 20:03 ` Jon TURNEY
2013-02-07 5:06 ` Christopher Faylor
0 siblings, 1 reply; 12+ messages in thread
From: Jon TURNEY @ 2013-02-06 20:03 UTC (permalink / raw)
To: cygwin-apps; +Cc: Jon TURNEY
- 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
2013-02-01 Jon TURNEY <jon.turney@dronecode.org.uk>
* install.cc ( _custom_MessageBox): Remove custom message box.
(FileInuseDlgProc): Add file-in-use dialog box.
(installOne): Use processlist to list processes using a file, and
offer to kill them with the file-in-use dialog.
* res.rc (IDD_FILE_INUSE) : New dialog.
* resource.h (IDD_FILE_INUSE, IDC_FILE_INUSE_EDIT)
(IDC_FILE_INUSE_MSG, IDC_FILE_INUSE_HELP): Define corresponding
resource ID numbers.
* processlist.h: New file.
* processlist.cc: New file.
* Makefile.am (setup_LDADD): Add -lpsapi.
(setup_SOURCES): Add new files.
---
Makefile.am | 4 +-
install.cc | 152 ++++++++++++++++++++++++----------
processlist.cc | 250 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
processlist.h | 41 +++++++++
res.rc | 20 +++++
resource.h | 4 +
6 files changed, 424 insertions(+), 47 deletions(-)
create mode 100644 processlist.cc
create mode 100644 processlist.h
diff --git a/Makefile.am b/Makefile.am
index 0f1498b..ddd19ed 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -105,7 +105,7 @@ inilint_SOURCES = \
setup_LDADD = \
libgetopt++/libgetopt++.la -lgcrypt -lgpg-error \
- -lshlwapi -lcomctl32 -lole32 -lwsock32 -lnetapi32 -luuid -llzma -lbz2 -lz
+ -lshlwapi -lcomctl32 -lole32 -lwsock32 -lnetapi32 -lpsapi -luuid -llzma -lbz2 -lz
setup_LDFLAGS = -mwindows -Wc,-static -static-libtool-libs
setup_SOURCES = \
AntiVirus.cc \
@@ -230,6 +230,8 @@ setup_SOURCES = \
postinstallresults.h \
prereq.cc \
prereq.h \
+ processlist.cc \
+ processlist.h \
proppage.cc \
proppage.h \
propsheet.cc \
diff --git a/install.cc b/install.cc
index 9d39f33..a838aff 100644
--- a/install.cc
+++ b/install.cc
@@ -63,6 +63,7 @@ static const char *cvsid = "\n%%% $Id: install.cc,v 2.101 2011/07/25 14:36:24 jt
#include "threebar.h"
#include "Exception.h"
+#include "processlist.h"
using namespace std;
@@ -192,39 +193,61 @@ Installer::replaceOnRebootSucceeded (const std::string& fn, bool &rebootneeded)
rebootneeded = true;
}
-#define MB_RETRYCONTINUE 7
-#if !defined(IDCONTINUE)
-#define IDCONTINUE IDCANCEL
-#endif
+typedef struct
+{
+ const char *msg;
+ const char *processlist;
+ int iteration;
+} FileInuseDlgData;
-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);
-}
+static BOOL CALLBACK
+FileInuseDlgProc (HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam)
+{
+ switch (uMsg)
+ {
+ case WM_INITDIALOG:
+ {
+ FileInuseDlgData *dlg_data = (FileInuseDlgData *)lParam;
+
+ SetDlgItemText (hwndDlg, IDC_FILE_INUSE_MSG, dlg_data->msg);
+ SetDlgItemText (hwndDlg, IDC_FILE_INUSE_EDIT, dlg_data->processlist);
+
+ switch (dlg_data->iteration)
+ {
+ case 0:
+ break; // show the dialog the way it is in the resource
+
+ case 1:
+ SetDlgItemText (hwndDlg, IDRETRY, "&Kill Processes");
+ SetDlgItemText (hwndDlg, IDC_FILE_INUSE_HELP,
+ "Select 'Kill' to kill Cygwin processes and retry, or "
+ "select 'Continue' to go on anyway (you will need to reboot).");
+ break;
+
+ default:
+ case 2:
+ SetDlgItemText (hwndDlg, IDRETRY, "&Kill Processes");
+ SetDlgItemText (hwndDlg, IDC_FILE_INUSE_HELP,
+ "Select 'Kill' to forcibly kill all processes and retry, or "
+ "select 'Continue' to go on anyway (you will need to reboot).");
+ }
+ }
+ return TRUE; // automatically set focus, please
-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;
+ case WM_COMMAND:
+ if (HIWORD (wParam) == BN_CLICKED)
+ {
+ switch (LOWORD (wParam))
+ {
+ case IDRETRY:
+ case IDOK:
+ EndDialog (hwndDlg, LOWORD (wParam));
+ return TRUE;
+ }
+ }
+ }
+
+ return FALSE;
}
/* Helper function to create the registry value "AllowProtectedRenames",
@@ -316,9 +339,6 @@ Installer::extract_replace_on_reboot (archive *tarstream, const std::string& pre
return false;
}
-#undef MessageBox
-#define MessageBox _custom_MessageBox
-
static char all_null[512];
/* install one source at a given prefix. */
@@ -427,7 +447,7 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
}
bool error_in_this_package = false;
- bool ignoreInUseErrors = unattended_mode;
+ bool ignoreInUseErrors = false;
bool ignoreExtractErrors = unattended_mode;
package_bytes = source.size;
@@ -448,7 +468,7 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
if (Script::isAScript (fn))
pkgm.desired.addScript (Script (canonicalfn));
- bool firstIteration = true;
+ int iteration = 0;
int extract_error = 0;
while ((extract_error = archive::extract_file (tarstream, prefixURL, prefixPath)) != 0)
{
@@ -458,21 +478,61 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
{
if (!ignoreInUseErrors)
{
- char msg[fn.size() + 300];
- sprintf (msg,
- "%snable to extract /%s -- the file is in use.\r\n"
- "Please stop %s Cygwin processes and select \"Retry\", or\r\n"
- "select \"Continue\" to go on anyway (you will need to reboot).\r\n",
- firstIteration?"U":"Still u", fn.c_str(), firstIteration?"all":"ALL");
+ // convert the file name to long UNC form
+ std::string s = backslash (cygpath ("/" + fn));
+ WCHAR sname[s.size () + 7];
+ mklongpath (sname, s.c_str (), s.size () + 7);
+
+ // find any process which has that file loaded into it
+ // (note that this doesn't find when the file is un-writeable because the process has
+ // that file opened exclusively)
+ ProcessList processes = Process::listProcessesWithModuleLoaded (sname);
+
+ std::string plm;
+ for (ProcessList::iterator i = processes.begin (); i != processes.end (); i++)
+ {
+ if (i != processes.begin ()) plm += "\r\n";
- switch (MessageBox (owner, msg, "In-use files detected",
- MB_RETRYCONTINUE | MB_ICONWARNING | MB_TASKMODAL))
+ std::string processName = i->getName ();
+ log (LOG_BABBLE) << processName << endLog;
+ plm += processName;
+ }
+
+ INT_PTR rc = (iteration < 3) ? IDRETRY : IDOK;
+ 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;
+
+ rc = DialogBoxParam(hinstance, MAKEINTRESOURCE (IDD_FILE_INUSE), owner, FileInuseDlgProc, (LPARAM)&dlg_data);
+ }
+
+ 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++)
+ {
+ processes = Process::listProcessesWithModuleLoaded (sname);
+ if (processes.size () == 0)
+ break;
+
+ Sleep (1000);
+ }
+
// retry
- firstIteration = false;
+ iteration++;
continue;
- case IDCONTINUE:
+ case IDOK:
+ // ignore this in-use error, and any subsequent in-use errors for other files in the same package
ignoreInUseErrors = true;
break;
default:
diff --git a/processlist.cc b/processlist.cc
new file mode 100644
index 0000000..e3363f9
--- /dev/null
+++ b/processlist.cc
@@ -0,0 +1,250 @@
+/*
+ * Copyright (c) 2013 Jon TURNEY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * A copy of the GNU General Public License can be found at
+ * http://www.gnu.org/
+ *
+ */
+
+#include <windows.h>
+#define PSAPI_VERSION 1
+#include <psapi.h>
+#include <stdio.h>
+
+#include "processlist.h"
+#include <String++.h>
+#include "LogSingleton.h"
+#include "script.h"
+#include "mount.h"
+#include "filemanip.h"
+
+// ---------------------------------------------------------------------------
+// implements class Process
+//
+// access to a Windows process
+// ---------------------------------------------------------------------------
+
+Process::Process (DWORD pid) : processID (pid)
+{
+}
+
+std::string
+Process::getName (void)
+{
+ HANDLE hProcess = OpenProcess (PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, processID);
+ char modName[MAX_PATH];
+ GetModuleFileNameExA (hProcess, NULL, modName, sizeof (modName));
+ CloseHandle (hProcess);
+
+ std::string s = modName;
+ s += " (pid " + stringify (processID) + ")";
+ return s;
+}
+
+DWORD
+Process::getProcessID (void)
+{
+ return processID;
+}
+
+void
+Process::kill (int force)
+{
+ if (force >= 2)
+ {
+ // use TerminateProcess() to request force-killing of the process
+ HANDLE hProcess = OpenProcess (PROCESS_TERMINATE, FALSE, processID);
+
+ if (hProcess)
+ {
+ if (TerminateProcess(hProcess, (UINT)-1))
+ {
+ log (LOG_BABBLE) << "TerminateProcess succeeded on pid " << processID << endLog;
+ }
+ else
+ {
+ log (LOG_BABBLE) << "TerminateProcess failed " << GetLastError () << " for pid " << processID << endLog;
+ }
+ CloseHandle (hProcess);
+ }
+
+ return;
+ }
+
+ std::string signame;
+
+ switch (force)
+ {
+ case 0:
+ signame = "-TERM";
+ break;
+
+ default:
+ case 1:
+ signame = "-KILL";
+ break;
+ }
+
+ std::string kill_cmd = backslash (cygpath ("/bin/kill.exe")) + " " + signame + " " + stringify (processID);
+ ::run (kill_cmd.c_str ());
+}
+
+//
+// test if a module is loaded into a process
+//
+bool
+Process::isModuleLoadedInProcess (const WCHAR *moduleName)
+{
+ BOOL match = FALSE;
+
+ // Get process handle
+ HANDLE hProcess = OpenProcess (PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, processID);
+
+ if (NULL == hProcess)
+ return FALSE;
+
+ static unsigned int bytesAllocated = 0;
+ static HMODULE *hMods = 0;
+
+ // initial allocation
+ if (bytesAllocated == 0)
+ {
+ bytesAllocated = sizeof (HMODULE)*1024;
+ hMods = (HMODULE *)malloc (bytesAllocated);
+ }
+
+ while (1)
+ {
+ DWORD cbNeeded;
+
+ // Get a list of all the modules in this process.
+ if (!EnumProcessModules (hProcess, hMods, bytesAllocated, &cbNeeded))
+ {
+ // Don't log ERROR_PARTIAL_COPY as expected for System process and those of different bitness
+ if (GetLastError () != ERROR_PARTIAL_COPY)
+ {
+ log (LOG_BABBLE) << "EnumProcessModules failed " << GetLastError () << " for pid " << processID << endLog;
+ }
+
+ cbNeeded = 0;
+ }
+
+ // If we didn't get all modules, retry with a larger array
+ if (cbNeeded > bytesAllocated)
+ {
+ bytesAllocated = cbNeeded;
+ hMods = (HMODULE *)realloc (hMods, bytesAllocated);
+ continue;
+ }
+
+ // Search module list for the module we are looking for
+ for (int i = 0; i < (cbNeeded / sizeof (HMODULE)); i++ )
+ {
+ WCHAR szModName[MAX_PATH];
+
+ // Get the full path to the module's file.
+ if (GetModuleFileNameExW (hProcess, hMods[i], szModName, sizeof (szModName)/sizeof (WCHAR)))
+ {
+ WCHAR canonicalModName[MAX_PATH];
+
+ // Canonicalise returned module name to long UNC form
+ if (wcscmp (szModName, L"\\\\?\\") != 0)
+ {
+ wcscpy (canonicalModName, L"\\\\?\\");
+ wcscat (canonicalModName, szModName);
+ }
+ else
+ {
+ wcscpy (canonicalModName, szModName);
+ }
+
+ // Does it match the name ?
+ if (wcscmp (moduleName, canonicalModName) == 0)
+ {
+ match = TRUE;
+ break;
+ }
+ }
+ }
+
+ break;
+ }
+
+ // Release the process handle
+ CloseHandle (hProcess);
+
+ return match;
+}
+
+//
+// get a list of currently running processes
+//
+ProcessList
+Process::snapshot (void)
+{
+ static DWORD *pProcessIDs = 0;
+ static unsigned int bytesAllocated = 0;
+ DWORD bytesReturned;
+
+ // initial allocation
+ if (bytesAllocated == 0)
+ {
+ bytesAllocated = sizeof (DWORD);
+ pProcessIDs = (DWORD *)malloc (bytesAllocated);
+ }
+
+ // fetch a snapshot of process list
+ while (1)
+ {
+ if (!EnumProcesses (pProcessIDs, bytesAllocated, &bytesReturned))
+ {
+ log (LOG_BABBLE) << "EnumProcesses failed " << GetLastError () << endLog;
+ bytesReturned = 0;
+ }
+
+ // If we didn't get all processes, retry with a larger array
+ if (bytesReturned == bytesAllocated)
+ {
+ bytesAllocated = bytesAllocated*2;
+ pProcessIDs = (DWORD *)realloc (pProcessIDs, bytesAllocated);
+ continue;
+ }
+
+ break;
+ }
+
+ // convert to ProcessList vector
+ unsigned int nProcesses = bytesReturned/sizeof (DWORD);
+ ProcessList v(nProcesses, 0);
+ for (unsigned int i = 0; i < nProcesses; i++)
+ {
+ v[i] = pProcessIDs[i];
+ }
+
+ return v;
+}
+
+//
+// list processes which have a given executable module loaded
+//
+ProcessList
+Process::listProcessesWithModuleLoaded (const WCHAR *moduleName)
+{
+ ProcessList v;
+ ProcessList pl = snapshot ();
+
+ for (ProcessList::iterator i = pl.begin (); i != pl.end (); i++)
+ {
+ if (i->isModuleLoadedInProcess (moduleName))
+ {
+ v.push_back (*i);
+ }
+ }
+
+ return v;
+}
diff --git a/processlist.h b/processlist.h
new file mode 100644
index 0000000..ef734a3
--- /dev/null
+++ b/processlist.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2013 Jon TURNEY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * A copy of the GNU General Public License can be found at
+ * http://www.gnu.org/
+ *
+ */
+
+#include <windows.h>
+#include <vector>
+#include <string>
+
+// ---------------------------------------------------------------------------
+// interface to class Process
+//
+// access to a Windows process
+// ---------------------------------------------------------------------------
+
+class Process;
+
+// utility type ProcessList, a vector of Process
+typedef std::vector<Process> ProcessList;
+
+class Process
+{
+public:
+ Process (DWORD pid);
+ std::string getName (void);
+ DWORD getProcessID (void);
+ void kill (int force);
+ bool isModuleLoadedInProcess (const WCHAR *moduleName);
+ static ProcessList listProcessesWithModuleLoaded (const WCHAR *moduleName);
+private:
+ DWORD processID;
+ static ProcessList snapshot (void);
+};
diff --git a/res.rc b/res.rc
index d6c0ff0..4bc8de1 100644
--- a/res.rc
+++ b/res.rc
@@ -440,6 +440,26 @@ BEGIN
END
+IDD_FILE_INUSE DIALOG DISCARDABLE 0, 0, SETUP_SMALL_DIALOG_DIMS
+STYLE DS_MODALFRAME | DS_CENTER | WS_POPUP | WS_CAPTION
+CAPTION "In-use file detected"
+FONT 8, "MS Shell Dlg"
+BEGIN
+ ICON IDI_WARNING,IDC_HEADICON,5,5
+ LTEXT "Unable to extract %s",
+ IDC_FILE_INUSE_MSG,27,5,183,8,SS_PATHELLIPSIS
+ LTEXT "The file is in use by the following processes:",
+ IDC_STATIC,27,14,183,8
+ EDITTEXT IDC_FILE_INUSE_EDIT,27,23,183,28,WS_VSCROLL |
+ ES_LEFT | ES_MULTILINE | ES_READONLY |
+ ES_AUTOVSCROLL | NOT WS_TABSTOP
+ LTEXT "Select 'Stop' to stop Cygwin processes and retry, or "
+ "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
+END
+
/////////////////////////////////////////////////////////////////////////////
//
// Manifest
diff --git a/resource.h b/resource.h
index df68473..99a6f42 100644
--- a/resource.h
+++ b/resource.h
@@ -64,6 +64,7 @@
#define IDD_PREREQ 220
#define IDD_DROPPED 221
#define IDD_POSTINSTALL 222
+#define IDD_FILE_INUSE 223
// Bitmaps
@@ -173,3 +174,6 @@
#define IDC_CHOOSE_CLEAR_SEARCH 587
#define IDC_LOCAL_DIR_DESC 588
#define IDC_POSTINSTALL_EDIT 589
+#define IDC_FILE_INUSE_EDIT 590
+#define IDC_FILE_INUSE_MSG 591
+#define IDC_FILE_INUSE_HELP 592
--
1.7.9
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH setup 0/2] List and offer to kill processes preventing a file from being written
2013-02-05 17:51 ` Christopher Faylor
@ 2013-02-06 20:07 ` Jon TURNEY
0 siblings, 0 replies; 12+ messages in thread
From: Jon TURNEY @ 2013-02-06 20:07 UTC (permalink / raw)
To: cygwin-apps
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH setup 2/2] List and offer to kill processes preventing a file from being written
2013-02-06 20:03 ` Jon TURNEY
@ 2013-02-07 5:06 ` Christopher Faylor
2013-07-23 22:41 ` Jon TURNEY
0 siblings, 1 reply; 12+ messages in thread
From: Christopher Faylor @ 2013-02-07 5:06 UTC (permalink / raw)
To: cygwin-apps
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.
Thanks.
cgf
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH setup 2/2] List and offer to kill processes preventing a file from being written
2013-02-07 5:06 ` Christopher Faylor
@ 2013-07-23 22:41 ` Jon TURNEY
2013-07-24 5:23 ` Christopher Faylor
0 siblings, 1 reply; 12+ messages in thread
From: Jon TURNEY @ 2013-07-23 22:41 UTC (permalink / raw)
To: cygwin-apps
[-- Attachment #1: Type: text/plain, Size: 2376 bytes --]
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.
[-- Attachment #2: inuse.patch --]
[-- Type: text/plain, Size: 7342 bytes --]
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
/////////////////////////////////////////////////////////////////////////////
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH setup 2/2] List and offer to kill processes preventing a file from being written
2013-07-23 22:41 ` Jon TURNEY
@ 2013-07-24 5:23 ` Christopher Faylor
0 siblings, 0 replies; 12+ messages in thread
From: Christopher Faylor @ 2013-07-24 5:23 UTC (permalink / raw)
To: cygwin-apps
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-07-24 5:23 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-05 15:26 [PATCH setup 0/2] List and offer to kill processes preventing a file from being written Jon TURNEY
2013-02-05 15:26 ` [PATCH setup 2/2] " Jon TURNEY
2013-02-06 20:03 ` Jon TURNEY
2013-02-07 5:06 ` Christopher Faylor
2013-07-23 22:41 ` Jon TURNEY
2013-07-24 5:23 ` Christopher Faylor
2013-02-05 15:26 ` [PATCH setup 1/2] Refactor ::run() so it's more generally useful Jon TURNEY
2013-02-05 16:07 ` [PATCH setup 0/2] List and offer to kill processes preventing a file from being written Christopher Faylor
2013-02-05 16:16 ` Corinna Vinschen
2013-02-05 17:42 ` Jon TURNEY
2013-02-05 17:51 ` Christopher Faylor
2013-02-06 20:07 ` Jon TURNEY
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).