[PATCH setup 0/2] List and offer to kill processes preventing a file from being written
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
[PATCH setup 1/2] Refactor ::run() so it's more generally useful
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.tur...@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.tur...@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.runXXX;
+ 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.postinstallXXX;
- 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
[PATCH setup 2/2] List and offer to kill processes preventing a file from being written
- 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.tur...@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.tur...@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
Re: [PATCH setup 0/2] List and offer to kill processes preventing a file from being written
Wow. Ambitious! On Tue, Feb 05, 2013 at 03:24:48PM +, 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
Re: [PATCH setup 0/2] List and offer to kill processes preventing a file from being written
On Feb 5 11:06, Christopher Faylor wrote: Wow. Ambitious! On Tue, Feb 05, 2013 at 03:24:48PM +, 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
nuke cygwin legacy?
Corinna +1'ed my suggestion that it was time to remove cygwin 1.5 support so I'm wondering if anyone has any objections to removing 1.5 from cygwin.com. I was going to suggest this a few months ago and mention that the Cygwin Time Machine was an alternative but it looks like that service is no longer available. So, as an alternative, we could advertise that the directory is going away on the main web page for a month before nuking it. cgf
Re: [PATCH setup 0/2] List and offer to kill processes preventing a file from being written
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 +, 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.
Re: nuke cygwin legacy?
Am 05.02.2013 18:41, schrieb Christopher Faylor: Corinna +1'ed my suggestion that it was time to remove cygwin 1.5 support so I'm wondering if anyone has any objections to removing 1.5 from cygwin.com. I was going to suggest this a few months ago and mention that the Cygwin Time Machine was an alternative but it looks like that service is no longer available. So, as an alternative, we could advertise that the directory is going away on the main web page for a month before nuking it. cgf I had thought that support had stopped anyway, just keeping the files available. I used to run an old laptop with Windows ME until recently and maybe there's an ME or Windows 2000 surviving in some virtual machine... For owners of old hardware, there is also some Linux installations still available that fit into something like 20MB, so why not keep cygwin for download? Thomas
Re: nuke cygwin legacy?
On 2/5/2013 10:41, Christopher Faylor wrote: Corinna +1'ed my suggestion that it was time to remove cygwin 1.5 support so I'm wondering if anyone has any objections to removing 1.5 from cygwin.com. It seems to me that the sort of person who's still hanging onto a DOS-based version of Windows probably won't be watching this list, or to the cygwin.com home page. I propose making the deprecation a two-stage affair: 1. Move Cygwin 1.5 somewhere else, so that it doesn't get included in mirrors. 2. Point everything referring to the old mirror system at the new location, presumably a different subtree on sourceware.org. Then you can rely on the FTP/HTTP logs to determine how often people actually install these packages. There's a core assumption here, which is that download volumes have dropped enough that going back to a single download server is sane. If it turns out that download volume is unexpectedly high, though, well, that's answer enough, isn't it?
Re: nuke cygwin legacy?
On Tue, 5 Feb 2013 13:00:43 -0500, Christopher Faylor wrote: Because it is taking space on the web site and on sourceware.org and there is no good reason for it to be offered anymore. If there are virtual machines running ME then, if they haven't installed Cygwin 1.5 by now, there is no reason to assume that they will need it. Additionally, except for setup.exe, there have been no bug fixes or security fixes in 1.5 so we're offering buggy, insecure software on our site. Which, in and of itself, is reason to drop it. FWIW, I deleted legacy files from Ports a few months ago without complaint. Yaakov
Re: nuke cygwin legacy?
On Tue, Feb 05, 2013 at 07:57:58PM -0600, Yaakov wrote: On Tue, 5 Feb 2013 13:00:43 -0500, Christopher Faylor wrote: Because it is taking space on the web site and on sourceware.org and there is no good reason for it to be offered anymore. If there are virtual machines running ME then, if they haven't installed Cygwin 1.5 by now, there is no reason to assume that they will need it. Additionally, except for setup.exe, there have been no bug fixes or security fixes in 1.5 so we're offering buggy, insecure software on our site. Which, in and of itself, is reason to drop it. FWIW, I deleted legacy files from Ports a few months ago without complaint. Also, FWIW, it looks like setup-legacy was downloaded 14 times in the last year. I think that's a pretty good indication that it isn't very popular. cgf
Re: nuke cygwin legacy?
On Tue, 05 Feb 2013 20:56:42 +0100, Erwin Waterlander wrote: It doesn't matter that it is not secure. Yes, it does. IMHO it is irresponsible on our part to distribute unmaintained or knowingly vulnerable software, and it reflects badly on the Cygwin project. Yaakov
