Merge pull request #1 from secondlife/develop

Fix UI issues, improve warnings, and enhance process handling

3 files changed, 369 insertions, 3 deletions

diff --git a/indra/llcommon/llapr.cpp b/indra/llcommon/llapr.cpp
index eeff2694a7..06eb4c389f 100644
--- a/indra/llcommon/llapr.cpp
+++ b/indra/llcommon/llapr.cpp
@@ -231,9 +231,35 @@ bool _ll_apr_warn_status(apr_status_t status, const char* file, int line)
 {
     if(APR_SUCCESS == status) return false;
 #if !LL_LINUX
-    char buf[MAX_STRING];   /* Flawfinder: ignore */
+    char buf[MAX_STRING];
     apr_strerror(status, buf, sizeof(buf));
-    LL_WARNS("APR") << "APR: " << file << ":" << line << " " << buf << LL_ENDL;
+
+#ifdef LL_WINDOWS
+    // On Windows, APR error strings may be in the system's ANSI code page (e.g., Cyrillic)
+    // Convert to UTF-8 for proper logging
+    std::string error_msg = buf;
+    int wlen = MultiByteToWideChar(CP_ACP, 0, buf, -1, nullptr, 0);
+    if (wlen > 0)
+    {
+        std::wstring wbuf(wlen, L'\0');
+        MultiByteToWideChar(CP_ACP, 0, buf, -1, &wbuf[0], wlen);
+
+        int utf8len = WideCharToMultiByte(CP_UTF8, 0, wbuf.c_str(), -1, nullptr, 0, nullptr, nullptr);
+        if (utf8len > 0)
+        {
+            std::string utf8buf(utf8len, '\0');
+            WideCharToMultiByte(CP_UTF8, 0, wbuf.c_str(), -1, &utf8buf[0], utf8len, nullptr, nullptr);
+            error_msg = utf8buf.c_str(); // Remove null terminator
+        }
+        LL_WARNS("APR") << "APR: " << file << ":" << line << " " << error_msg << " (0x" << std::hex << status << std::dec << ")" << LL_ENDL;
+    }
+    else
+    {
+        LL_WARNS("APR") << "APR: " << file << ":" << line << " " << buf << " (0x" << std::hex << status << std::dec << ")" << LL_ENDL;
+    }
+#else
+    LL_WARNS("APR") << "APR: " << file << ":" << line << " " << buf << " (0x" << std::hex << status << std::dec << ")" << LL_ENDL;
+#endif
 #endif
     return true;
 }
diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp
index 730c85ef5f..a71589a2bf 100644
--- a/indra/llcommon/llprocess.cpp
+++ b/indra/llcommon/llprocess.cpp
@@ -63,6 +63,87 @@
 #include <sys/wait.h>
 #endif
 
+#if LL_WINDOWS
+#include <windows.h>
+#include "llwin32headers.h"
+#include <mutex>
+
+namespace {
+    // Global job object that will kill all child processes when parent terminates
+    HANDLE g_jobObject = NULL;
+    bool g_jobObjectInitialized = false;
+    std::mutex g_jobObjectMutex;
+
+    void InitializeJobObject()
+    {
+        if (g_jobObjectInitialized)
+            return;
+        std::lock_guard<std::mutex> lock(g_jobObjectMutex);
+        if (g_jobObjectInitialized)
+            return;
+
+        g_jobObjectInitialized = true;
+
+        // Create a job object
+        g_jobObject = ::CreateJobObjectW(NULL, NULL);
+        if (g_jobObject == NULL)
+        {
+            LL_WARNS("LLProcess") << "Failed to create job object: " << ::GetLastError() << LL_ENDL;
+            return;
+        }
+
+        // Configure the job to kill all processes when the last handle closes
+        // (i.e., when the parent process exits)
+        JOBOBJECT_EXTENDED_LIMIT_INFORMATION jeli = { 0 };
+        jeli.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
+
+        // Windows 8+ supports nested jobs (for VS studio)
+        jeli.BasicLimitInformation.LimitFlags |= JOB_OBJECT_LIMIT_BREAKAWAY_OK;
+
+        if (!::SetInformationJobObject(g_jobObject, JobObjectExtendedLimitInformation,
+            &jeli, sizeof(jeli)))
+        {
+            LL_WARNS("LLProcess") << "Failed to set job object limits: " << ::GetLastError() << LL_ENDL;
+            ::CloseHandle(g_jobObject);
+            g_jobObject = NULL;
+            return;
+        }
+
+        LL_INFOS("LLProcess") << "Job object created - child processes will terminate with parent" << LL_ENDL;
+    }
+
+    void AssignProcessToJob(HANDLE hProcess, const std::string& desc)
+    {
+        if (!g_jobObjectInitialized)
+            InitializeJobObject();
+
+        if (g_jobObject != NULL)
+        {
+            if (!::AssignProcessToJobObject(g_jobObject, hProcess))
+            {
+                DWORD error = ::GetLastError();
+                // ERROR_ACCESS_DENIED (5) means the process is already in a job
+                // This can happen if the parent viewer is itself in a job
+                if (error == ERROR_ACCESS_DENIED)
+                {
+                    LL_WARNS("LLProcess") << "Autokill requested but process " << desc
+                        << " is already in a job object (ERROR_ACCESS_DENIED)"
+                        << LL_ENDL;
+                }
+                else
+                {
+                    LL_WARNS("LLProcess") << "Failed to assign process " << desc
+                        << " to job object: error " << error << LL_ENDL;
+                }
+            }
+            else
+            {
+                LL_DEBUGS("LLProcess") << "Process " << desc << " assigned to job object" << LL_ENDL;
+            }
+        }
+    }
+}
+#endif
 
 namespace bp = boost::process::v2;
 namespace asio = boost::asio;
@@ -653,7 +734,16 @@ void LLProcess::launch(const LLSDOrParams& params)
                 << ": " << ex.what()));
         }
 
-#if !LL_WINDOWS
+#if LL_WINDOWS
+        // Add the process to the job object so it terminates when parent dies.
+        // This is done for all processes with autokill=true (the default).
+        // Job objects are the Windows-recommended way to ensure child processes
+        // don't become orphaned if the parent crashes or is killed.
+        if (mAutokill && mChild)
+        {
+            AssignProcessToJob(mChild->native_handle(), mDesc);
+        }
+#else
         // boost::process v2 may install a SIGCHLD handler via boost::asio
         // without SA_RESTART when mIOContext is passed to bp::process.
         // Without SA_RESTART, blocking waitpid() calls elsewhere in the
diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp
index 84f6c131e7..00dcd52d5d 100644
--- a/indra/llcommon/tests/llprocess_test.cpp
+++ b/indra/llcommon/tests/llprocess_test.cpp
@@ -110,6 +110,19 @@ static std::string readfile(const std::string& pathname, const std::string& desc
     return output;
 }
 
+#if LL_WINDOWS
+static std::string readfile_if_present(const std::string& pathname)
+{
+    std::ifstream inf(pathname.c_str());
+    if (!inf.is_open())
+    {
+        return "";
+    }
+    return std::string((std::istreambuf_iterator<char>(inf)),
+                       std::istreambuf_iterator<char>());
+}
+#endif
+
 /// Looping on LLProcess::isRunning() must now be accompanied by pumping
 /// "mainloop" -- otherwise the status won't update and you get an infinite
 /// loop.
@@ -251,6 +264,215 @@ struct PythonProcessLauncher
     NamedExtTempFile mScript;
 };
 
+#if LL_WINDOWS
+namespace
+{
+    static constexpr const char* AUTOKILL_HELPER_SCRIPT_ENV = "LLPROCESS_AUTOKILL_HELPER_SCRIPT";
+    static constexpr const char* AUTOKILL_HELPER_PIDFILE_ENV = "LLPROCESS_AUTOKILL_HELPER_PIDFILE";
+    static constexpr const char* AUTOKILL_HELPER_RELEASE_ENV = "LLPROCESS_AUTOKILL_HELPER_RELEASE";
+    static constexpr const char* AUTOKILL_HELPER_COUNT_ENV = "LLPROCESS_AUTOKILL_HELPER_COUNT";
+    static constexpr int AUTOKILL_HELPER_RELEASE_TIMEOUT_SECONDS = 60;
+    static constexpr int AUTOKILL_HELPER_PID_TIMEOUT_SECONDS = 15;
+    static constexpr DWORD AUTOKILL_HELPER_POLL_INTERVAL_MS = 100;
+    static constexpr DWORD AUTOKILL_CHILD_TERMINATION_TIMEOUT_MS = 5000;
+    static constexpr int AUTOKILL_HELPER_INVALID_ENV_EXIT = 2;
+    static constexpr int AUTOKILL_HELPER_INVALID_COUNT_EXIT = 3;
+    static constexpr int AUTOKILL_HELPER_LAUNCH_FAILURE_EXIT = 4;
+
+    struct ScopedEnvironmentVariable
+    {
+        ScopedEnvironmentVariable(const char* name, const std::string& value):
+            mName(name),
+            mHadValue(false)
+        {
+            DWORD size = GetEnvironmentVariableA(name, nullptr, 0);
+            if (size > 0)
+            {
+                std::vector<char> buffer(size);
+                DWORD copied = GetEnvironmentVariableA(name, buffer.data(), size);
+                if (copied > 0)
+                {
+                    mHadValue = true;
+                    mOldValue.assign(buffer.data(), copied);
+                }
+            }
+            SetEnvironmentVariableA(name, value.c_str());
+        }
+
+        ~ScopedEnvironmentVariable()
+        {
+            SetEnvironmentVariableA(mName.c_str(), mHadValue ? mOldValue.c_str() : nullptr);
+        }
+
+        std::string mName;
+        std::string mOldValue;
+        bool mHadValue;
+    };
+
+    std::string get_current_executable_path()
+    {
+        std::vector<char> buffer(MAX_PATH);
+        for (;;)
+        {
+            DWORD length = GetModuleFileNameA(nullptr, buffer.data(), static_cast<DWORD>(buffer.size()));
+            tut::ensure("GetModuleFileNameA() failed", length > 0);
+            if (length < buffer.size() &&
+                (length < buffer.size() - 1 || buffer[length] == '\0'))
+            {
+                return std::string(buffer.data(), length);
+            }
+            buffer.resize(buffer.size() * 2);
+        }
+    }
+
+    void run_autokill_helper_from_environment()
+    {
+        const std::string script = LLStringUtil::getenv(AUTOKILL_HELPER_SCRIPT_ENV);
+        if (script.empty())
+        {
+            return;
+        }
+
+        const std::string pidfile = LLStringUtil::getenv(AUTOKILL_HELPER_PIDFILE_ENV);
+        const std::string releasefile = LLStringUtil::getenv(AUTOKILL_HELPER_RELEASE_ENV);
+        const std::string countstr = LLStringUtil::getenv(AUTOKILL_HELPER_COUNT_ENV);
+        const std::string python = LLStringUtil::getenv("PYTHON");
+        int child_count = 1;
+        if (!countstr.empty())
+        {
+            try
+            {
+                child_count = std::stoi(countstr);
+            }
+            catch (const std::exception& err)
+            {
+                LL_WARNS("LLProcess") << "Invalid autokill helper child count '"
+                                       << countstr << "': " << err.what() << LL_ENDL;
+                std::exit(AUTOKILL_HELPER_INVALID_COUNT_EXIT);
+            }
+        }
+
+        if (pidfile.empty() || releasefile.empty() || python.empty() || child_count < 1)
+        {
+            std::exit(AUTOKILL_HELPER_INVALID_ENV_EXIT);
+        }
+
+        std::vector<LLProcessPtr> children;
+        children.reserve(child_count);
+        std::ofstream out(pidfile.c_str(), std::ios::trunc);
+        for (int i = 0; i < child_count; ++i)
+        {
+            LLProcess::Params params;
+            params.executable = python;
+            params.args.add(script);
+            params.autokill = true;
+            params.attached = false;
+            LLProcessPtr child = LLProcess::create(params);
+            if (!child)
+            {
+                std::exit(AUTOKILL_HELPER_LAUNCH_FAILURE_EXIT);
+            }
+            children.push_back(child);
+            out << child->getProcessID() << '\n';
+        }
+        out.flush();
+        out.close();
+
+        for (DWORD elapsed_ms = 0;
+             elapsed_ms < AUTOKILL_HELPER_RELEASE_TIMEOUT_SECONDS * 1000;
+             elapsed_ms += AUTOKILL_HELPER_POLL_INTERVAL_MS)
+        {
+            if (readfile_if_present(releasefile) == "exit")
+            {
+                break;
+            }
+            Sleep(AUTOKILL_HELPER_POLL_INTERVAL_MS);
+        }
+
+        // Exit the helper process itself so the job handle closes and Windows
+        // terminates the autokilled children.
+        std::exit(0);
+    }
+
+    std::vector<DWORD> wait_for_helper_pids(
+        const std::string& pidfile,
+        int expected_count,
+        int timeout = AUTOKILL_HELPER_PID_TIMEOUT_SECONDS)
+    {
+        for (int i = 0;
+             i < (timeout * 1000) / static_cast<int>(AUTOKILL_HELPER_POLL_INTERVAL_MS);
+             ++i)
+        {
+            std::ifstream inf(pidfile.c_str());
+            std::vector<DWORD> pids;
+            DWORD pid = 0;
+            while (inf >> pid)
+            {
+                pids.push_back(pid);
+            }
+            if (static_cast<int>(pids.size()) == expected_count)
+            {
+                return pids;
+            }
+            Sleep(AUTOKILL_HELPER_POLL_INTERVAL_MS);
+            LLEventPumps::instance().obtain("mainloop").post(LLSD());
+        }
+        tut::ensure(STRINGIZE("expected " << expected_count
+                                << " child pids within " << timeout
+                                << " seconds"), false);
+        return {};
+    }
+
+    void verify_autokill_on_helper_exit(const std::string& desc, int child_count)
+    {
+        NamedExtTempFile child_script("py",
+            "import time\n"
+            "time.sleep(30)\n");
+        NamedTempFile pidfile("pid", "");
+        NamedTempFile releasefile("release", "");
+
+        ScopedEnvironmentVariable helper_script(AUTOKILL_HELPER_SCRIPT_ENV, child_script.getName());
+        ScopedEnvironmentVariable helper_pidfile(AUTOKILL_HELPER_PIDFILE_ENV, pidfile.getName());
+        ScopedEnvironmentVariable helper_release(AUTOKILL_HELPER_RELEASE_ENV, releasefile.getName());
+        ScopedEnvironmentVariable helper_count(AUTOKILL_HELPER_COUNT_ENV, std::to_string(child_count));
+
+        LLProcess::Params params;
+        params.executable = get_current_executable_path();
+        params.desc = desc + " helper";
+        LLProcessPtr helper = LLProcess::create(params);
+        tut::ensure("helper launched", bool(helper));
+
+        std::vector<DWORD> pids = wait_for_helper_pids(pidfile.getName(), child_count);
+        std::vector<HANDLE> handles;
+        handles.reserve(pids.size());
+        for (DWORD pid : pids)
+        {
+            // SYNCHRONIZE lets the test wait for the child to terminate, while
+            // PROCESS_QUERY_LIMITED_INFORMATION keeps the requested access minimal.
+            HANDLE handle = OpenProcess(SYNCHRONIZE | PROCESS_QUERY_LIMITED_INFORMATION, false, pid);
+            tut::ensure(STRINGIZE("opened child process handle for pid " << pid), handle != nullptr);
+            handles.push_back(handle);
+        }
+
+        {
+            std::ofstream out(releasefile.getName().c_str(), std::ios::trunc);
+            out << "exit";
+        }
+
+        waitfor(*helper);
+        tut::ensure_equals("helper exited", helper->getStatus().mState, LLProcess::EXITED);
+
+        for (HANDLE handle : handles)
+        {
+            tut::ensure_equals("autokilled child exited",
+                               WaitForSingleObject(handle, AUTOKILL_CHILD_TERMINATION_TIMEOUT_MS),
+                               WAIT_OBJECT_0);
+            CloseHandle(handle);
+        }
+    }
+}
+#endif
+
 /// convenience function for PythonProcessLauncher::run()
 template <typename CONTENT>
 static void python(const std::string& desc, const CONTENT& script)
@@ -303,6 +525,13 @@ namespace tut
 {
     struct llprocess_data
     {
+        llprocess_data()
+        {
+#if LL_WINDOWS
+            run_autokill_helper_from_environment();
+#endif
+        }
+
         LLAPRPool pool;
     };
     typedef test_group<llprocess_data> llprocess_group;
@@ -1815,4 +2044,25 @@ namespace tut
                elapsed_ms < 75);
     }
 
+    template<> template<>
+    void object::test<36>()
+    {
+        set_test_name("autokill ensures child termination on parent exit");
+#if !LL_WINDOWS
+        skip("Windows-specific test");
+#else
+        verify_autokill_on_helper_exit(get_test_name(), 1);
+#endif
+    }
+
+    template<> template<>
+    void object::test<37>()
+    {
+        set_test_name("multiple processes with autokill");
+#if !LL_WINDOWS
+        skip("Windows-specific test");
+#else
+        verify_autokill_on_helper_exit(get_test_name(), 2);
+#endif
+    }
 } // namespace tut