From c0731c1c05cafe508c91c5f583301234ba3b8403 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 17 Jan 2012 18:55:42 -0500 Subject: Add log message if LLProcessLauncher child fails to execv(). On a Posix platform (vfork()/execv() implementation), if for any reason the execv() failed (e.g. executable not on PATH), the viewer would never know, nor the user: the vfork() child produced no output, and terminated with rc 0! Add logging, make child terminate with nonzero rc. Remove pointless addArgument(const char*) overload: this does nothing for you that the compiler won't do implicitly. In llupdateinstaller.cpp, remove pointless c_str() call in addArgument() arg: we were starting with a std::string, then extracting its c_str(), only to construct a whole new std::string from it! --- indra/llcommon/llprocesslauncher.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'indra/llcommon/llprocesslauncher.h') diff --git a/indra/llcommon/llprocesslauncher.h b/indra/llcommon/llprocesslauncher.h index 954c249147..1daa980c58 100644 --- a/indra/llcommon/llprocesslauncher.h +++ b/indra/llcommon/llprocesslauncher.h @@ -28,6 +28,7 @@ #define LL_LLPROCESSLAUNCHER_H #if LL_WINDOWS +#define WIN32_LEAN_AND_MEAN #include #endif @@ -51,7 +52,6 @@ public: void clearArguments(); void addArgument(const std::string &arg); - void addArgument(const char *arg); int launch(void); bool isRunning(void); @@ -66,10 +66,12 @@ public: void orphan(void); // This needs to be called periodically on Mac/Linux to clean up zombie processes. + // (However, as of 2012-01-12 there are no such calls in the viewer code base. :-P ) static void reap(void); // Accessors for platform-specific process ID #if LL_WINDOWS + // (Windows flavor unused as of 2012-01-12) HANDLE getProcessHandle() { return mProcessHandle; }; #else pid_t getProcessID() { return mProcessID; }; -- cgit v1.2.3 From 06f9dbd8db9895f81d7bd325d8cf616f68533396 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 18 Jan 2012 01:09:50 -0500 Subject: Introduce static LLProcessLauncher::isRunning(ll_pid_t) method. typedef LLProcessLauncher::ll_pid_t to be HANDLE on Windows, pid_t elsewhere. Then we can define getProcessID() returning ll_pid_t on all platforms, retaining getProcessHandle() for hypothetical existing consumers... of which there are none in practice. This lets us define isRunning(ll_pid_t) to encapsulate the platform-specific logic to actually check on a running child process, turning non-static isRunning() into a fairly trivial wrapper. --- indra/llcommon/llprocesslauncher.h | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/llprocesslauncher.h') diff --git a/indra/llcommon/llprocesslauncher.h b/indra/llcommon/llprocesslauncher.h index 1daa980c58..63193abd8f 100644 --- a/indra/llcommon/llprocesslauncher.h +++ b/indra/llcommon/llprocesslauncher.h @@ -54,6 +54,8 @@ public: void addArgument(const std::string &arg); int launch(void); + // isRunning isn't const because, if child isn't running, it clears stored + // process ID bool isRunning(void); // Attempt to kill the process -- returns true if the process is no longer running when it returns. @@ -72,10 +74,23 @@ public: // Accessors for platform-specific process ID #if LL_WINDOWS // (Windows flavor unused as of 2012-01-12) - HANDLE getProcessHandle() { return mProcessHandle; }; + typedef HANDLE ll_pid_t; + HANDLE getProcessHandle() const { return mProcessHandle; } + ll_pid_t getProcessID() const { return mProcessHandle; } #else - pid_t getProcessID() { return mProcessID; }; + typedef pid_t ll_pid_t; + ll_pid_t getProcessID() const { return mProcessID; }; #endif + /** + * Test if a process (ll_pid_t obtained from getProcessID()) is still + * running. Return is same nonzero ll_pid_t value if still running, else + * zero, so you can test it like a bool. But if you want to update a + * stored variable as a side effect, you can write code like this: + * @code + * childpid = LLProcessLauncher::isRunning(childpid); + * @endcode + */ + static ll_pid_t isRunning(ll_pid_t); private: std::string mExecutable; -- cgit v1.2.3 From f0dbb878337082d3f581874c12e6df2f4659a464 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 20 Jan 2012 18:10:40 -0500 Subject: Per Richard, replace LLProcessLauncher with LLProcess. LLProcessLauncher had the somewhat fuzzy mandate of (1) accumulating parameters with which to launch a child process and (2) sometimes tracking the lifespan of the ensuing child process. But a valid LLProcessLauncher object might or might not have ever been associated with an actual child process. LLProcess specifically tracks a child process. In effect, it's a fairly thin wrapper around a process HANDLE (on Windows) or pid_t (elsewhere), with lifespan management thrown in. A static LLProcess::create() method launches a new child; create() accepts an LLSD bundle with child parameters. So building up a parameter bundle is deferred to LLSD rather than conflated with the process management object. Reconcile all known LLProcessLauncher consumers in the viewer code base, notably the class unit tests. --- indra/llcommon/llprocesslauncher.h | 107 ------------------------------------- 1 file changed, 107 deletions(-) delete mode 100644 indra/llcommon/llprocesslauncher.h (limited to 'indra/llcommon/llprocesslauncher.h') diff --git a/indra/llcommon/llprocesslauncher.h b/indra/llcommon/llprocesslauncher.h deleted file mode 100644 index 63193abd8f..0000000000 --- a/indra/llcommon/llprocesslauncher.h +++ /dev/null @@ -1,107 +0,0 @@ -/** - * @file llprocesslauncher.h - * @brief Utility class for launching, terminating, and tracking the state of processes. - * - * $LicenseInfo:firstyear=2008&license=viewerlgpl$ - * Second Life Viewer Source Code - * Copyright (C) 2010, Linden Research, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; - * version 2.1 of the License only. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - * - * Linden Research, Inc., 945 Battery Street, San Francisco, CA 94111 USA - * $/LicenseInfo$ - */ - -#ifndef LL_LLPROCESSLAUNCHER_H -#define LL_LLPROCESSLAUNCHER_H - -#if LL_WINDOWS -#define WIN32_LEAN_AND_MEAN -#include -#endif - - -/* - LLProcessLauncher handles launching external processes with specified command line arguments. - It also keeps track of whether the process is still running, and can kill it if required. -*/ - -class LL_COMMON_API LLProcessLauncher -{ - LOG_CLASS(LLProcessLauncher); -public: - LLProcessLauncher(); - virtual ~LLProcessLauncher(); - - void setExecutable(const std::string &executable); - void setWorkingDirectory(const std::string &dir); - - const std::string& getExecutable() const; - - void clearArguments(); - void addArgument(const std::string &arg); - - int launch(void); - // isRunning isn't const because, if child isn't running, it clears stored - // process ID - bool isRunning(void); - - // Attempt to kill the process -- returns true if the process is no longer running when it returns. - // Note that even if this returns false, the process may exit some time after it's called. - bool kill(void); - - // Use this if you want the external process to continue execution after the LLProcessLauncher instance controlling it is deleted. - // Normally, the destructor will attempt to kill the process and wait for termination. - // This should only be used if the viewer is about to exit -- otherwise, the child process will become a zombie after it exits. - void orphan(void); - - // This needs to be called periodically on Mac/Linux to clean up zombie processes. - // (However, as of 2012-01-12 there are no such calls in the viewer code base. :-P ) - static void reap(void); - - // Accessors for platform-specific process ID -#if LL_WINDOWS - // (Windows flavor unused as of 2012-01-12) - typedef HANDLE ll_pid_t; - HANDLE getProcessHandle() const { return mProcessHandle; } - ll_pid_t getProcessID() const { return mProcessHandle; } -#else - typedef pid_t ll_pid_t; - ll_pid_t getProcessID() const { return mProcessID; }; -#endif - /** - * Test if a process (ll_pid_t obtained from getProcessID()) is still - * running. Return is same nonzero ll_pid_t value if still running, else - * zero, so you can test it like a bool. But if you want to update a - * stored variable as a side effect, you can write code like this: - * @code - * childpid = LLProcessLauncher::isRunning(childpid); - * @endcode - */ - static ll_pid_t isRunning(ll_pid_t); - -private: - std::string mExecutable; - std::string mWorkingDir; - std::vector mLaunchArguments; - -#if LL_WINDOWS - HANDLE mProcessHandle; -#else - pid_t mProcessID; -#endif -}; - -#endif // LL_LLPROCESSLAUNCHER_H -- cgit v1.2.3