From 14c09c3ce597e47f5c41bb45246e89a1f31d89b0 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 21 Dec 2011 10:06:26 -0500 Subject: Add unit-test module for LLProcessLauncher. As always with llcommon, this is expressed as an "integration test" to sidestep a circular dependency: the llcommon build depends on its unit tests, but all our unit tests depend on llcommon. Initial test code is more for human verification than automated verification: does APR's child-process management in fact support nonblocking operations? --- indra/llcommon/tests/llprocesslauncher_test.cpp | 144 ++++++++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 indra/llcommon/tests/llprocesslauncher_test.cpp (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp new file mode 100644 index 0000000000..3b5602f620 --- /dev/null +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -0,0 +1,144 @@ +/** + * @file llprocesslauncher_test.cpp + * @author Nat Goodspeed + * @date 2011-12-19 + * @brief Test for llprocesslauncher. + * + * $LicenseInfo:firstyear=2011&license=viewerlgpl$ + * Copyright (c) 2011, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llprocesslauncher.h" +// STL headers +#include +// std headers +// external library headers +#include "llapr.h" +#include "apr_thread_proc.h" +#include "apr_file_io.h" +// other Linden headers +#include "../test/lltut.h" + +class APR +{ +public: + APR(): + pool(NULL) + { + apr_initialize(); + apr_pool_create(&pool, NULL); + } + + ~APR() + { + apr_terminate(); + } + + std::string strerror(apr_status_t rv) + { + char errbuf[256]; + apr_strerror(rv, errbuf, sizeof(errbuf)); + return errbuf; + } + + apr_pool_t *pool; +}; + +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + struct llprocesslauncher_data + { + void aprchk(apr_status_t rv) + { + ensure_equals(apr.strerror(rv), rv, APR_SUCCESS); + } + + APR apr; + }; + typedef test_group llprocesslauncher_group; + typedef llprocesslauncher_group::object object; + llprocesslauncher_group llprocesslaunchergrp("llprocesslauncher"); + + template<> template<> + void object::test<1>() + { + set_test_name("raw APR nonblocking I/O"); + + apr_procattr_t *procattr = NULL; + aprchk(apr_procattr_create(&procattr, apr.pool)); + aprchk(apr_procattr_io_set(procattr, APR_CHILD_BLOCK, APR_CHILD_BLOCK, APR_CHILD_BLOCK)); + aprchk(apr_procattr_cmdtype_set(procattr, APR_PROGRAM_PATH)); + + std::vector argv; + apr_proc_t child; + argv.push_back("python"); + argv.push_back("-c"); + argv.push_back("raise RuntimeError('Hello from Python!')"); + argv.push_back(NULL); + + aprchk(apr_proc_create(&child, argv[0], + static_cast(&argv[0]), + NULL, // if we wanted to pass explicit environment + procattr, + apr.pool)); + + typedef std::pair DescFile; + typedef std::vector DescFileVec; + DescFileVec outfiles; + outfiles.push_back(DescFile("out", child.out)); + outfiles.push_back(DescFile("err", child.err)); + + while (! outfiles.empty()) + { + DescFileVec iterfiles(outfiles); + for (size_t i = 0; i < iterfiles.size(); ++i) + { + char buf[4096]; + + apr_status_t rv = apr_file_gets(buf, sizeof(buf), iterfiles[i].second); + if (APR_STATUS_IS_EOF(rv)) + { + std::cout << "(EOF on " << iterfiles[i].first << ")\n"; + outfiles.erase(outfiles.begin() + i); + continue; + } + if (rv != APR_SUCCESS) + { + std::cout << "(waiting; apr_file_gets(" << iterfiles[i].first << ") => " << rv << ": " << apr.strerror(rv) << ")\n"; + continue; + } + // Is it even possible to get APR_SUCCESS but read 0 bytes? + // Hope not, but defend against that anyway. + if (buf[0]) + { + std::cout << iterfiles[i].first << ": " << buf; + // Just for pretty output... if we only read a partial + // line, terminate it. + if (buf[strlen(buf) - 1] != '\n') + std::cout << "...\n"; + } + } + sleep(1); + } + apr_file_close(child.in); + apr_file_close(child.out); + apr_file_close(child.err); + + int rc = 0; + apr_exit_why_e why; + apr_status_t rv; + while (! APR_STATUS_IS_CHILD_DONE(rv = apr_proc_wait(&child, &rc, &why, APR_NOWAIT))) + { + std::cout << "child not done (" << rv << "): " << apr.strerror(rv) << '\n'; + sleep(0.5); + } + std::cout << "child done: rv = " << rv << " (" << apr.strerror(rv) << "), why = " << why << ", rc = " << rc << '\n'; + } +} // namespace tut -- cgit v1.2.3 From 7832d8eccb00d32b6122e5851238e962f65af1e8 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 21 Dec 2011 11:12:48 -0500 Subject: Fix llprocesslauncher_test.cpp to work on Windows. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index 3b5602f620..4d14e1be53 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -12,6 +12,7 @@ // Precompiled header #include "linden_common.h" // associated header +#define WIN32_LEAN_AND_MEAN #include "llprocesslauncher.h" // STL headers #include @@ -23,6 +24,10 @@ // other Linden headers #include "../test/lltut.h" +#if defined(LL_WINDOWS) +#define sleep _sleep +#endif + class APR { public: -- cgit v1.2.3 From 2fd0bc8648e71aa2f141fde4b3a6a0165f7ef4d6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 21 Dec 2011 17:00:43 -0500 Subject: Change llprocesslauncher_test.cpp eyeballing to program verification. That is, where before we just flung stuff to stdout with the expectation that a human user would verify, replace with assertions in the test code itself. Quiet previous noise on stdout. Introduce a temp script file that produces output on both stdout and stderr, with sleep() calls so we predictably have to wait for it. Track and then verify the history of our interaction with the child process, noting especially EWOULDBLOCK attempts. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 97 ++++++++++++++++++++++--- 1 file changed, 85 insertions(+), 12 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index 4d14e1be53..ca06b3164e 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -17,6 +17,7 @@ // STL headers #include // std headers +#include // external library headers #include "llapr.h" #include "apr_thread_proc.h" @@ -71,11 +72,53 @@ namespace tut typedef llprocesslauncher_group::object object; llprocesslauncher_group llprocesslaunchergrp("llprocesslauncher"); + struct Item + { + Item(): tries(0) {} + unsigned tries; + std::string which; + std::string what; + }; + template<> template<> void object::test<1>() { set_test_name("raw APR nonblocking I/O"); + // Create a script file in a temporary place. + const char* tempdir = NULL; + aprchk(apr_temp_dir_get(&tempdir, apr.pool)); + + // Construct a temp filename template in that directory. + char *tempname = NULL; + aprchk(apr_filepath_merge(&tempname, tempdir, "testXXXXXX", 0, apr.pool)); + + // Create a temp file from that template. + apr_file_t* fp = NULL; + aprchk(apr_file_mktemp(&fp, tempname, APR_CREATE | APR_WRITE | APR_EXCL, apr.pool)); + + // Write it. + const char script[] = + "import sys\n" + "import time\n" + "\n" + "time.sleep(2)\n" + "print >>sys.stdout, \"stdout after wait\"\n" + "sys.stdout.flush()\n" + "time.sleep(2)\n" + "print >>sys.stderr, \"stderr after wait\"\n" + "sys.stderr.flush()\n" + ; + apr_size_t len(sizeof(script)-1); + aprchk(apr_file_write(fp, script, &len)); + aprchk(apr_file_close(fp)); + + // Arrange to track the history of our interaction with child: what we + // fetched, which pipe it came from, how many tries it took before we + // got it. + std::vector history; + history.push_back(Item()); + apr_procattr_t *procattr = NULL; aprchk(apr_procattr_create(&procattr, apr.pool)); aprchk(apr_procattr_io_set(procattr, APR_CHILD_BLOCK, APR_CHILD_BLOCK, APR_CHILD_BLOCK)); @@ -84,8 +127,7 @@ namespace tut std::vector argv; apr_proc_t child; argv.push_back("python"); - argv.push_back("-c"); - argv.push_back("raise RuntimeError('Hello from Python!')"); + argv.push_back(tempname); argv.push_back(NULL); aprchk(apr_proc_create(&child, argv[0], @@ -110,24 +152,35 @@ namespace tut apr_status_t rv = apr_file_gets(buf, sizeof(buf), iterfiles[i].second); if (APR_STATUS_IS_EOF(rv)) { - std::cout << "(EOF on " << iterfiles[i].first << ")\n"; +// std::cout << "(EOF on " << iterfiles[i].first << ")\n"; + history.back().which = iterfiles[i].first; + history.back().what = "*eof*"; + history.push_back(Item()); outfiles.erase(outfiles.begin() + i); continue; } - if (rv != APR_SUCCESS) + if (rv == EWOULDBLOCK) { - std::cout << "(waiting; apr_file_gets(" << iterfiles[i].first << ") => " << rv << ": " << apr.strerror(rv) << ")\n"; +// std::cout << "(waiting; apr_file_gets(" << iterfiles[i].first << ") => " << rv << ": " << apr.strerror(rv) << ")\n"; + ++history.back().tries; continue; } + ensure_equals(rv, APR_SUCCESS); // Is it even possible to get APR_SUCCESS but read 0 bytes? // Hope not, but defend against that anyway. if (buf[0]) { - std::cout << iterfiles[i].first << ": " << buf; - // Just for pretty output... if we only read a partial - // line, terminate it. - if (buf[strlen(buf) - 1] != '\n') - std::cout << "...\n"; +// std::cout << iterfiles[i].first << ": " << buf; + history.back().which = iterfiles[i].first; + history.back().what.append(buf); + if (buf[strlen(buf) - 1] == '\n') + history.push_back(Item()); + else + { + // Just for pretty output... if we only read a partial + // line, terminate it. +// std::cout << "...\n"; + } } } sleep(1); @@ -141,9 +194,29 @@ namespace tut apr_status_t rv; while (! APR_STATUS_IS_CHILD_DONE(rv = apr_proc_wait(&child, &rc, &why, APR_NOWAIT))) { - std::cout << "child not done (" << rv << "): " << apr.strerror(rv) << '\n'; +// std::cout << "child not done (" << rv << "): " << apr.strerror(rv) << '\n'; sleep(0.5); } - std::cout << "child done: rv = " << rv << " (" << apr.strerror(rv) << "), why = " << why << ", rc = " << rc << '\n'; +// std::cout << "child done: rv = " << rv << " (" << apr.strerror(rv) << "), why = " << why << ", rc = " << rc << '\n'; + ensure_equals(rv, APR_CHILD_DONE); + ensure_equals(why, APR_PROC_EXIT); + ensure_equals(rc, 0); + + // Remove temp script file + aprchk(apr_file_remove(tempname, apr.pool)); + + // Beyond merely executing all the above successfully, verify that we + // obtained expected output -- and that we duly got control while + // waiting, proving the non-blocking nature of these pipes. + ensure("blocking I/O on child pipe (0)", history[0].tries); + ensure_equals(history[0].which, "out"); + ensure_equals(history[0].what, "stdout after wait\n"); + ensure("blocking I/O on child pipe (1)", history[1].tries); + ensure_equals(history[1].which, "out"); + ensure_equals(history[1].what, "*eof*"); + ensure_equals(history[2].which, "err"); + ensure_equals(history[2].what, "stderr after wait\n"); + ensure_equals(history[3].which, "err"); + ensure_equals(history[3].what, "*eof*"); } } // namespace tut -- cgit v1.2.3 From 25ef0cd2236aeb2d0047881e11a0022c4355cd48 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 21 Dec 2011 20:42:11 -0500 Subject: Tweak llprocesslauncher_test.cpp to run properly on Windows. Fix EOL issues: "\r\n" vs. "\n". On Windows, requesting a read in nonblocking mode can produce EAGAIN instead of EWOULDBLOCK. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 27 ++++++++++++++----------- 1 file changed, 15 insertions(+), 12 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index ca06b3164e..bdae81770f 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -27,6 +27,9 @@ #if defined(LL_WINDOWS) #define sleep _sleep +#define EOL "\r\n" +#else +#define EOL "\n" #endif class APR @@ -99,15 +102,15 @@ namespace tut // Write it. const char script[] = - "import sys\n" - "import time\n" - "\n" - "time.sleep(2)\n" - "print >>sys.stdout, \"stdout after wait\"\n" - "sys.stdout.flush()\n" - "time.sleep(2)\n" - "print >>sys.stderr, \"stderr after wait\"\n" - "sys.stderr.flush()\n" + "import sys" EOL + "import time" EOL + EOL + "time.sleep(2)" EOL + "print >>sys.stdout, \"stdout after wait\"" EOL + "sys.stdout.flush()" EOL + "time.sleep(2)" EOL + "print >>sys.stderr, \"stderr after wait\"" EOL + "sys.stderr.flush()" EOL ; apr_size_t len(sizeof(script)-1); aprchk(apr_file_write(fp, script, &len)); @@ -159,7 +162,7 @@ namespace tut outfiles.erase(outfiles.begin() + i); continue; } - if (rv == EWOULDBLOCK) + if (rv == EWOULDBLOCK || rv == EAGAIN) { // std::cout << "(waiting; apr_file_gets(" << iterfiles[i].first << ") => " << rv << ": " << apr.strerror(rv) << ")\n"; ++history.back().tries; @@ -210,12 +213,12 @@ namespace tut // waiting, proving the non-blocking nature of these pipes. ensure("blocking I/O on child pipe (0)", history[0].tries); ensure_equals(history[0].which, "out"); - ensure_equals(history[0].what, "stdout after wait\n"); + ensure_equals(history[0].what, "stdout after wait" EOL); ensure("blocking I/O on child pipe (1)", history[1].tries); ensure_equals(history[1].which, "out"); ensure_equals(history[1].what, "*eof*"); ensure_equals(history[2].which, "err"); - ensure_equals(history[2].what, "stderr after wait\n"); + ensure_equals(history[2].what, "stderr after wait" EOL); ensure_equals(history[3].which, "err"); ensure_equals(history[3].what, "*eof*"); } -- cgit v1.2.3 From 39c3efbda3bc4c7b415aa851ec4f42f05acda0cb Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 22 Dec 2011 16:20:19 -0500 Subject: Add child_status_callback() function and arrange to call periodically. At least on OS X 10.7, a call to apr_proc_wait(APR_NOWAIT) in fact seems to block the caller. So instead of polling apr_proc_wait(), use APR callback mechanism (apr_proc_other_child_register() et al.) and poll that using apr_proc_other_child_refresh_all(). Evidently this polls the underlying system waitpid(), but the internal call seems to better support nonblocking. On arrival in the child_status_callback(APR_OC_REASON_DEATH) call, though, apr_proc_wait() produces ECHILD: the child process in question has already been reaped. The OS-encoded wait() status does get passed to the callback, but then we have to use OS-dependent macros to tease apart voluntary termination vs. killed by signal... a bit of a hole in APR's abstraction layer. Wrap ensure_equals() calls with a macro to explain which comparison failed. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 161 ++++++++++++++++++++---- 1 file changed, 138 insertions(+), 23 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index bdae81770f..7d67d13960 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -22,14 +22,17 @@ #include "llapr.h" #include "apr_thread_proc.h" #include "apr_file_io.h" +#include // other Linden headers #include "../test/lltut.h" +#include "stringize.h" #if defined(LL_WINDOWS) #define sleep _sleep #define EOL "\r\n" #else #define EOL "\n" +#include #endif class APR @@ -57,6 +60,10 @@ public: apr_pool_t *pool; }; +#define ensure_equals_(left, right) \ + ensure_equals(STRINGIZE(#left << " != " << #right), (left), (right)) +#define aprchk(expr) aprchk_(#expr, (expr)) + /***************************************************************************** * TUT *****************************************************************************/ @@ -64,9 +71,10 @@ namespace tut { struct llprocesslauncher_data { - void aprchk(apr_status_t rv) + void aprchk_(const char* call, apr_status_t rv) { - ensure_equals(apr.strerror(rv), rv, APR_SUCCESS); + ensure_equals(STRINGIZE(call << " => " << rv << ": " << apr.strerror(rv)), + rv, APR_SUCCESS); } APR apr; @@ -83,6 +91,91 @@ namespace tut std::string what; }; +#define tabent(symbol) { symbol, #symbol } + static struct ReasonCode + { + int code; + const char* name; + } reasons[] = + { + tabent(APR_OC_REASON_DEATH), + tabent(APR_OC_REASON_UNWRITABLE), + tabent(APR_OC_REASON_RESTART), + tabent(APR_OC_REASON_UNREGISTER), + tabent(APR_OC_REASON_LOST), + tabent(APR_OC_REASON_RUNNING) + }; +#undef tabent + + struct WaitInfo + { + WaitInfo(apr_proc_t* child_): + child(child_), + rv(-1), // we haven't yet called apr_proc_wait() + rc(0), + why(apr_exit_why_e(0)) + {} + apr_proc_t* child; // which subprocess + apr_status_t rv; // return from apr_proc_wait() + int rc; // child's exit code + apr_exit_why_e why; // APR_PROC_EXIT, APR_PROC_SIGNAL, APR_PROC_SIGNAL_CORE + }; + + void child_status_callback(int reason, void* data, int status) + { + std::string reason_str; + BOOST_FOREACH(const ReasonCode& rcp, reasons) + { + if (reason == rcp.code) + { + reason_str = rcp.name; + break; + } + } + if (reason_str.empty()) + { + reason_str = STRINGIZE("unknown reason " << reason); + } + std::cout << "child_status_callback(" << reason_str << ")\n"; + + if (reason == APR_OC_REASON_DEATH || reason == APR_OC_REASON_LOST) + { + // Somewhat oddly, APR requires that you explicitly unregister + // even when it already knows the child has terminated. + apr_proc_other_child_unregister(data); + + WaitInfo* wi(static_cast(data)); + wi->rv = apr_proc_wait(wi->child, &wi->rc, &wi->why, APR_NOWAIT); + if (wi->rv == ECHILD) + { + std::cout << "apr_proc_wait() got ECHILD during child_status_callback(" + << reason_str << ")\n"; + // So -- is this why we have a 'status' param? + wi->rv = APR_CHILD_DONE; // pretend this call worked; fake results +#if defined(LL_WINDOWS) + wi->why = APR_PROC_EXIT; + wi->rc = status; // correct?? +#else // Posix + if (WIFEXITED(status)) + { + wi->why = APR_PROC_EXIT; + wi->rc = WEXITSTATUS(status); + } + else if (WIFSIGNALED(status)) + { + wi->why = APR_PROC_SIGNAL; + wi->rc = WTERMSIG(status); + } + else // uh, shouldn't happen? + { + wi->why = APR_PROC_EXIT; + wi->rc = status; // someone else will have to decode + } +#endif // Posix + } + } + } + template<> template<> void object::test<1>() { @@ -106,10 +199,10 @@ namespace tut "import time" EOL EOL "time.sleep(2)" EOL - "print >>sys.stdout, \"stdout after wait\"" EOL + "print >>sys.stdout, 'stdout after wait'" EOL "sys.stdout.flush()" EOL "time.sleep(2)" EOL - "print >>sys.stderr, \"stderr after wait\"" EOL + "print >>sys.stderr, 'stderr after wait'" EOL "sys.stderr.flush()" EOL ; apr_size_t len(sizeof(script)-1); @@ -122,6 +215,7 @@ namespace tut std::vector history; history.push_back(Item()); + // Run the child process. apr_procattr_t *procattr = NULL; aprchk(apr_procattr_create(&procattr, apr.pool)); aprchk(apr_procattr_io_set(procattr, APR_CHILD_BLOCK, APR_CHILD_BLOCK, APR_CHILD_BLOCK)); @@ -134,11 +228,23 @@ namespace tut argv.push_back(NULL); aprchk(apr_proc_create(&child, argv[0], - static_cast(&argv[0]), + &argv[0], NULL, // if we wanted to pass explicit environment procattr, apr.pool)); + // We do not want this child process to outlive our APR pool. On + // destruction of the pool, forcibly kill the process. Tell APR to try + // SIGTERM and wait 3 seconds. If that didn't work, use SIGKILL. + apr_pool_note_subprocess(apr.pool, &child, APR_KILL_AFTER_TIMEOUT); + + // arrange to call child_status_callback() + WaitInfo wi(&child); + apr_proc_other_child_register(&child, child_status_callback, &wi, child.in, apr.pool); + + // Monitor two different output pipes. Because one will be closed + // before the other, keep them in a vector so we can drop whichever of + // them is closed first. typedef std::pair DescFile; typedef std::vector DescFileVec; DescFileVec outfiles; @@ -168,7 +274,7 @@ namespace tut ++history.back().tries; continue; } - ensure_equals(rv, APR_SUCCESS); + aprchk_("apr_file_gets(buf, sizeof(buf), iterfiles[i].second)", rv); // Is it even possible to get APR_SUCCESS but read 0 bytes? // Hope not, but defend against that anyway. if (buf[0]) @@ -186,24 +292,33 @@ namespace tut } } } + // Do this once per tick, as we expect the viewer will + apr_proc_other_child_refresh_all(APR_OC_REASON_RUNNING); sleep(1); } apr_file_close(child.in); apr_file_close(child.out); apr_file_close(child.err); - int rc = 0; - apr_exit_why_e why; - apr_status_t rv; - while (! APR_STATUS_IS_CHILD_DONE(rv = apr_proc_wait(&child, &rc, &why, APR_NOWAIT))) + // Okay, we've broken the loop because our pipes are all closed. If we + // haven't yet called wait, give the callback one more chance. This + // models the fact that unlike this small test program, the viewer + // will still be running. + if (wi.rv == -1) + { + std::cout << "last gasp apr_proc_other_child_refresh_all()\n"; + apr_proc_other_child_refresh_all(APR_OC_REASON_RUNNING); + } + + if (wi.rv == -1) { -// std::cout << "child not done (" << rv << "): " << apr.strerror(rv) << '\n'; - sleep(0.5); + std::cout << "child_status_callback() wasn't called\n"; + wi.rv = apr_proc_wait(wi.child, &wi.rc, &wi.why, APR_NOWAIT); } // std::cout << "child done: rv = " << rv << " (" << apr.strerror(rv) << "), why = " << why << ", rc = " << rc << '\n'; - ensure_equals(rv, APR_CHILD_DONE); - ensure_equals(why, APR_PROC_EXIT); - ensure_equals(rc, 0); + ensure_equals_(wi.rv, APR_CHILD_DONE); + ensure_equals_(wi.why, APR_PROC_EXIT); + ensure_equals_(wi.rc, 0); // Remove temp script file aprchk(apr_file_remove(tempname, apr.pool)); @@ -212,14 +327,14 @@ namespace tut // obtained expected output -- and that we duly got control while // waiting, proving the non-blocking nature of these pipes. ensure("blocking I/O on child pipe (0)", history[0].tries); - ensure_equals(history[0].which, "out"); - ensure_equals(history[0].what, "stdout after wait" EOL); + ensure_equals_(history[0].which, "out"); + ensure_equals_(history[0].what, "stdout after wait" EOL); ensure("blocking I/O on child pipe (1)", history[1].tries); - ensure_equals(history[1].which, "out"); - ensure_equals(history[1].what, "*eof*"); - ensure_equals(history[2].which, "err"); - ensure_equals(history[2].what, "stderr after wait" EOL); - ensure_equals(history[3].which, "err"); - ensure_equals(history[3].what, "*eof*"); + ensure_equals_(history[1].which, "out"); + ensure_equals_(history[1].what, "*eof*"); + ensure_equals_(history[2].which, "err"); + ensure_equals_(history[2].what, "stderr after wait" EOL); + ensure_equals_(history[3].which, "err"); + ensure_equals_(history[3].what, "*eof*"); } } // namespace tut -- cgit v1.2.3 From 6ccba8810102cc13def8057a82463c9787b21e57 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 22 Dec 2011 17:18:02 -0500 Subject: Never call apr_proc_wait() inside child_status_callback(). Quiet the temporary child_status_callback() output. Add a bit of diagnostic info if apr_proc_wait() returns anything but APR_CHILD_DONE. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 57 +++++++++++++------------ 1 file changed, 29 insertions(+), 28 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index 7d67d13960..bd7666313e 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -71,10 +71,10 @@ namespace tut { struct llprocesslauncher_data { - void aprchk_(const char* call, apr_status_t rv) + void aprchk_(const char* call, apr_status_t rv, apr_status_t expected=APR_SUCCESS) { ensure_equals(STRINGIZE(call << " => " << rv << ": " << apr.strerror(rv)), - rv, APR_SUCCESS); + rv, expected); } APR apr; @@ -123,6 +123,7 @@ namespace tut void child_status_callback(int reason, void* data, int status) { +/*==========================================================================*| std::string reason_str; BOOST_FOREACH(const ReasonCode& rcp, reasons) { @@ -137,6 +138,7 @@ namespace tut reason_str = STRINGIZE("unknown reason " << reason); } std::cout << "child_status_callback(" << reason_str << ")\n"; +|*==========================================================================*/ if (reason == APR_OC_REASON_DEATH || reason == APR_OC_REASON_LOST) { @@ -145,34 +147,33 @@ namespace tut apr_proc_other_child_unregister(data); WaitInfo* wi(static_cast(data)); - wi->rv = apr_proc_wait(wi->child, &wi->rc, &wi->why, APR_NOWAIT); - if (wi->rv == ECHILD) - { - std::cout << "apr_proc_wait() got ECHILD during child_status_callback(" - << reason_str << ")\n"; - // So -- is this why we have a 'status' param? - wi->rv = APR_CHILD_DONE; // pretend this call worked; fake results + // It's just wrong to call apr_proc_wait() here. The only way APR + // knows to call us with APR_OC_REASON_DEATH is that it's already + // reaped this child process, so calling wait() will only produce + // "huh?" from the OS. We must rely on the status param passed in, + // which unfortunately comes straight from the OS wait() call. +// wi->rv = apr_proc_wait(wi->child, &wi->rc, &wi->why, APR_NOWAIT); + wi->rv = APR_CHILD_DONE; // fake apr_proc_wait() results #if defined(LL_WINDOWS) - wi->why = APR_PROC_EXIT; - wi->rc = status; // correct?? + wi->why = APR_PROC_EXIT; + wi->rc = status; // no encoding on Windows (no signals) #else // Posix - if (WIFEXITED(status)) - { - wi->why = APR_PROC_EXIT; - wi->rc = WEXITSTATUS(status); - } - else if (WIFSIGNALED(status)) - { - wi->why = APR_PROC_SIGNAL; - wi->rc = WTERMSIG(status); - } - else // uh, shouldn't happen? - { - wi->why = APR_PROC_EXIT; - wi->rc = status; // someone else will have to decode - } -#endif // Posix + if (WIFEXITED(status)) + { + wi->why = APR_PROC_EXIT; + wi->rc = WEXITSTATUS(status); + } + else if (WIFSIGNALED(status)) + { + wi->why = APR_PROC_SIGNAL; + wi->rc = WTERMSIG(status); } + else // uh, shouldn't happen? + { + wi->why = APR_PROC_EXIT; + wi->rc = status; // someone else will have to decode + } +#endif // Posix } } @@ -316,7 +317,7 @@ namespace tut wi.rv = apr_proc_wait(wi.child, &wi.rc, &wi.why, APR_NOWAIT); } // std::cout << "child done: rv = " << rv << " (" << apr.strerror(rv) << "), why = " << why << ", rc = " << rc << '\n'; - ensure_equals_(wi.rv, APR_CHILD_DONE); + aprchk_("apr_proc_wait(wi->child, &wi->rc, &wi->why, APR_NOWAIT)", wi.rv, APR_CHILD_DONE); ensure_equals_(wi.why, APR_PROC_EXIT); ensure_equals_(wi.rc, 0); -- cgit v1.2.3 From 29273ffba68d254ce3e6d9939a854c778a377721 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 22 Dec 2011 17:27:53 -0500 Subject: Comment out lookup table used only by commented-out code. Otherwise the unreferenced declaration causes a fatal warning. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index bd7666313e..b3e0796191 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -91,6 +91,7 @@ namespace tut std::string what; }; +/*==========================================================================*| #define tabent(symbol) { symbol, #symbol } static struct ReasonCode { @@ -106,6 +107,7 @@ namespace tut tabent(APR_OC_REASON_RUNNING) }; #undef tabent +|*==========================================================================*/ struct WaitInfo { -- cgit v1.2.3 From cf7c6f93f28534fee2c13e29501b6ab6e7b77d61 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 23 Dec 2011 15:23:03 -0500 Subject: Make pipe-management logic more robust. Previous logic was vulnerable to the case in which both pipes reached EOF in the same loop iteration. Now we use std::list instead of std::vector, allowing us to iterate and delete with a single pass. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 43 ++++++++++++++++--------- 1 file changed, 27 insertions(+), 16 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index bd7666313e..d6d05ed769 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -16,6 +16,7 @@ #include "llprocesslauncher.h" // STL headers #include +#include // std headers #include // external library headers @@ -28,7 +29,7 @@ #include "stringize.h" #if defined(LL_WINDOWS) -#define sleep _sleep +#define sleep(secs) _sleep((secs) * 1000) #define EOL "\r\n" #else #define EOL "\n" @@ -244,44 +245,54 @@ namespace tut apr_proc_other_child_register(&child, child_status_callback, &wi, child.in, apr.pool); // Monitor two different output pipes. Because one will be closed - // before the other, keep them in a vector so we can drop whichever of + // before the other, keep them in a list so we can drop whichever of // them is closed first. typedef std::pair DescFile; - typedef std::vector DescFileVec; - DescFileVec outfiles; + typedef std::list DescFileList; + DescFileList outfiles; outfiles.push_back(DescFile("out", child.out)); outfiles.push_back(DescFile("err", child.err)); while (! outfiles.empty()) { - DescFileVec iterfiles(outfiles); - for (size_t i = 0; i < iterfiles.size(); ++i) + // This peculiar for loop is designed to let us erase(dfli). With + // a list, that invalidates only dfli itself -- but even so, we + // lose the ability to increment it for the next item. So at the + // top of every loop, while dfli is still valid, increment + // dflnext. Then before the next iteration, set dfli to dflnext. + for (DescFileList::iterator + dfli(outfiles.begin()), dflnext(outfiles.begin()), dflend(outfiles.end()); + dfli != dflend; dfli = dflnext) { + // Only valid to increment dflnext once we're sure it's not + // already at dflend. + ++dflnext; + char buf[4096]; - apr_status_t rv = apr_file_gets(buf, sizeof(buf), iterfiles[i].second); + apr_status_t rv = apr_file_gets(buf, sizeof(buf), dfli->second); if (APR_STATUS_IS_EOF(rv)) { -// std::cout << "(EOF on " << iterfiles[i].first << ")\n"; - history.back().which = iterfiles[i].first; +// std::cout << "(EOF on " << dfli->first << ")\n"; + history.back().which = dfli->first; history.back().what = "*eof*"; history.push_back(Item()); - outfiles.erase(outfiles.begin() + i); + outfiles.erase(dfli); continue; } if (rv == EWOULDBLOCK || rv == EAGAIN) { -// std::cout << "(waiting; apr_file_gets(" << iterfiles[i].first << ") => " << rv << ": " << apr.strerror(rv) << ")\n"; +// std::cout << "(waiting; apr_file_gets(" << dfli->first << ") => " << rv << ": " << apr.strerror(rv) << ")\n"; ++history.back().tries; continue; } - aprchk_("apr_file_gets(buf, sizeof(buf), iterfiles[i].second)", rv); + aprchk_("apr_file_gets(buf, sizeof(buf), dfli->second)", rv); // Is it even possible to get APR_SUCCESS but read 0 bytes? // Hope not, but defend against that anyway. if (buf[0]) { -// std::cout << iterfiles[i].first << ": " << buf; - history.back().which = iterfiles[i].first; +// std::cout << dfli->first << ": " << buf; + history.back().which = dfli->first; history.back().what.append(buf); if (buf[strlen(buf) - 1] == '\n') history.push_back(Item()); @@ -295,7 +306,7 @@ namespace tut } // Do this once per tick, as we expect the viewer will apr_proc_other_child_refresh_all(APR_OC_REASON_RUNNING); - sleep(1); + sleep(0.5); } apr_file_close(child.in); apr_file_close(child.out); @@ -313,7 +324,7 @@ namespace tut if (wi.rv == -1) { - std::cout << "child_status_callback() wasn't called\n"; + std::cout << "child_status_callback(APR_OC_REASON_DEATH) wasn't called" << std::endl; wi.rv = apr_proc_wait(wi.child, &wi.rc, &wi.why, APR_NOWAIT); } // std::cout << "child done: rv = " << rv << " (" << apr.strerror(rv) << "), why = " << why << ", rc = " << rc << '\n'; -- cgit v1.2.3 From 8008d540e5177aa4fb0c802b157eec2695c8334a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 23 Dec 2011 16:45:05 -0500 Subject: Should we expect EOF on one pipe before we finish reading the other? Defend test against the ambiguous answer to that question by not recording, or testing for, EOF history events. Enrich output for history-verification failures: display whole history array. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 61 +++++++++++++++++++------ 1 file changed, 48 insertions(+), 13 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index 895325c705..dbbe54e9fa 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -246,6 +246,11 @@ namespace tut WaitInfo wi(&child); apr_proc_other_child_register(&child, child_status_callback, &wi, child.in, apr.pool); + // TODO: + // Stuff child.in until it (would) block to verify EWOULDBLOCK/EAGAIN. + // Have child script clear it later, then write one more line to prove + // that it gets through. + // Monitor two different output pipes. Because one will be closed // before the other, keep them in a list so we can drop whichever of // them is closed first. @@ -276,9 +281,9 @@ namespace tut if (APR_STATUS_IS_EOF(rv)) { // std::cout << "(EOF on " << dfli->first << ")\n"; - history.back().which = dfli->first; - history.back().what = "*eof*"; - history.push_back(Item()); +// history.back().which = dfli->first; +// history.back().what = "*eof*"; +// history.push_back(Item()); outfiles.erase(dfli); continue; } @@ -340,15 +345,45 @@ namespace tut // Beyond merely executing all the above successfully, verify that we // obtained expected output -- and that we duly got control while // waiting, proving the non-blocking nature of these pipes. - ensure("blocking I/O on child pipe (0)", history[0].tries); - ensure_equals_(history[0].which, "out"); - ensure_equals_(history[0].what, "stdout after wait" EOL); - ensure("blocking I/O on child pipe (1)", history[1].tries); - ensure_equals_(history[1].which, "out"); - ensure_equals_(history[1].what, "*eof*"); - ensure_equals_(history[2].which, "err"); - ensure_equals_(history[2].what, "stderr after wait" EOL); - ensure_equals_(history[3].which, "err"); - ensure_equals_(history[3].what, "*eof*"); + try + { + unsigned i = 0; + ensure("blocking I/O on child pipe (0)", history[i].tries); + ensure_equals_(history[i].which, "out"); + ensure_equals_(history[i].what, "stdout after wait" EOL); +// ++i; +// ensure_equals_(history[i].which, "out"); +// ensure_equals_(history[i].what, "*eof*"); + ++i; + ensure("blocking I/O on child pipe (1)", history[i].tries); + ensure_equals_(history[i].which, "err"); + ensure_equals_(history[i].what, "stderr after wait" EOL); +// ++i; +// ensure_equals_(history[i].which, "err"); +// ensure_equals_(history[i].what, "*eof*"); + } + catch (const failure&) + { + std::cout << "History:\n"; + BOOST_FOREACH(const Item& item, history) + { + std::string what(item.what); + if ((! what.empty()) && what[what.length() - 1] == '\n') + { + what.erase(what.length() - 1); + if ((! what.empty()) && what[what.length() - 1] == '\r') + { + what.erase(what.length() - 1); + what.append("\\r"); + } + what.append("\\n"); + } + std::cout << " " << item.which << ": '" << what << "' (" + << item.tries << " tries)\n"; + } + std::cout << std::flush; + // re-raise same error; just want to enrich the output + throw; + } } } // namespace tut -- cgit v1.2.3 From 97876f6118eadf6a2669826d68412cc020975a64 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 23 Dec 2011 17:38:34 -0500 Subject: Fix sleep(0.5) to sleep(1) -- truncation to int makes that dubious. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index dbbe54e9fa..4d8f850d92 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -313,7 +313,7 @@ namespace tut } // Do this once per tick, as we expect the viewer will apr_proc_other_child_refresh_all(APR_OC_REASON_RUNNING); - sleep(0.5); + sleep(1); } apr_file_close(child.in); apr_file_close(child.out); -- cgit v1.2.3 From 61b5f3143e4ea53c9f64e5a1a5ad19f2edf3e776 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 5 Jan 2012 15:43:23 -0500 Subject: Introduce LLStreamQueue to buffer nonblocking I/O. Add unit tests to verify basic functionality. --- indra/llcommon/tests/llstreamqueue_test.cpp | 177 ++++++++++++++++++++++++++++ 1 file changed, 177 insertions(+) create mode 100644 indra/llcommon/tests/llstreamqueue_test.cpp (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llstreamqueue_test.cpp b/indra/llcommon/tests/llstreamqueue_test.cpp new file mode 100644 index 0000000000..e88c37d5be --- /dev/null +++ b/indra/llcommon/tests/llstreamqueue_test.cpp @@ -0,0 +1,177 @@ +/** + * @file llstreamqueue_test.cpp + * @author Nat Goodspeed + * @date 2012-01-05 + * @brief Test for llstreamqueue. + * + * $LicenseInfo:firstyear=2012&license=viewerlgpl$ + * Copyright (c) 2012, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llstreamqueue.h" +// STL headers +#include +// std headers +// external library headers +#include +// other Linden headers +#include "../test/lltut.h" +#include "stringize.h" + +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + struct llstreamqueue_data + { + llstreamqueue_data(): + // we want a buffer with actual bytes in it, not an empty vector + buffer(10) + {} + // As LLStreamQueue is merely a typedef for + // LLGenericStreamQueue, and no logic in LLGenericStreamQueue is + // specific to the instantiation, we're comfortable for now + // testing only the narrow-char version. + LLStreamQueue strq; + // buffer for use in multiple tests + std::vector buffer; + }; + typedef test_group llstreamqueue_group; + typedef llstreamqueue_group::object object; + llstreamqueue_group llstreamqueuegrp("llstreamqueue"); + + template<> template<> + void object::test<1>() + { + set_test_name("empty LLStreamQueue"); + ensure_equals("brand-new LLStreamQueue isn't empty", + strq.asSource().read(&buffer[0], buffer.size()), 0); + strq.asSink().close(); + ensure_equals("closed empty LLStreamQueue not at EOF", + strq.asSource().read(&buffer[0], buffer.size()), -1); + } + + template<> template<> + void object::test<2>() + { + set_test_name("one internal block, one buffer"); + LLStreamQueue::Sink sink(strq.asSink()); + ensure_equals("write(\"\")", sink.write("", 0), 0); + ensure_equals("0 write should leave LLStreamQueue empty", + strq.peek(&buffer[0], buffer.size()), 0); + // The meaning of "atomic" is that it must be smaller than our buffer. + std::string atomic("atomic"); + ensure("test data exceeds buffer", atomic.length() < buffer.size()); + ensure_equals(STRINGIZE("write(\"" << atomic << "\")"), + sink.write(&atomic[0], atomic.length()), atomic.length()); + size_t peeklen(strq.peek(&buffer[0], buffer.size())); + ensure_equals(STRINGIZE("peek(\"" << atomic << "\")"), + peeklen, atomic.length()); + ensure_equals(STRINGIZE("peek(\"" << atomic << "\") result"), + std::string(buffer.begin(), buffer.begin() + peeklen), atomic); + // peek() should not consume. Use a different buffer to prove it isn't + // just leftover data from the first peek(). + std::vector again(buffer.size()); + peeklen = size_t(strq.peek(&again[0], again.size())); + ensure_equals(STRINGIZE("peek(\"" << atomic << "\") again"), + peeklen, atomic.length()); + ensure_equals(STRINGIZE("peek(\"" << atomic << "\") again result"), + std::string(again.begin(), again.begin() + peeklen), atomic); + // now consume. + std::vector third(buffer.size()); + size_t readlen(strq.read(&third[0], third.size())); + ensure_equals(STRINGIZE("read(\"" << atomic << "\")"), + readlen, atomic.length()); + ensure_equals(STRINGIZE("read(\"" << atomic << "\") result"), + std::string(third.begin(), third.begin() + readlen), atomic); + ensure_equals("peek() after read()", strq.peek(&buffer[0], buffer.size()), 0); + } + + template<> template<> + void object::test<3>() + { + set_test_name("basic skip()"); + std::string lovecraft("lovecraft"); + ensure("test data exceeds buffer", lovecraft.length() < buffer.size()); + ensure_equals(STRINGIZE("write(\"" << lovecraft << "\")"), + strq.write(&lovecraft[0], lovecraft.length()), lovecraft.length()); + size_t peeklen(strq.peek(&buffer[0], buffer.size())); + ensure_equals(STRINGIZE("peek(\"" << lovecraft << "\")"), + peeklen, lovecraft.length()); + ensure_equals(STRINGIZE("peek(\"" << lovecraft << "\") result"), + std::string(buffer.begin(), buffer.begin() + peeklen), lovecraft); + std::streamsize skip1(4); + ensure_equals(STRINGIZE("skip(" << skip1 << ")"), strq.skip(skip1), skip1); + size_t readlen(strq.read(&buffer[0], buffer.size())); + ensure_equals(STRINGIZE("read(\"" << lovecraft.substr(skip1) << "\")"), + readlen, lovecraft.length() - skip1); + ensure_equals(STRINGIZE("read(\"" << lovecraft.substr(skip1) << "\") result"), + std::string(buffer.begin(), buffer.begin() + readlen), + lovecraft.substr(skip1)); + ensure_equals("unconsumed", strq.read(&buffer[0], buffer.size()), 0); + } + + template<> template<> + void object::test<4>() + { + set_test_name("skip() multiple blocks"); + std::string blocks[] = { "books of ", "H.P. ", "Lovecraft" }; + std::streamsize skip(blocks[0].length() + blocks[1].length() + 4); + BOOST_FOREACH(const std::string& block, blocks) + { + strq.write(&block[0], block.length()); + } + std::streamsize skiplen(strq.skip(skip)); + ensure_equals(STRINGIZE("skip(" << skip << ")"), skiplen, skip); + size_t readlen(strq.read(&buffer[0], buffer.size())); + ensure_equals("read(\"craft\")", readlen, 5); + ensure_equals("read(\"craft\") result", + std::string(buffer.begin(), buffer.begin() + readlen), "craft"); + } + + template<> template<> + void object::test<5>() + { + set_test_name("concatenate blocks"); + std::string blocks[] = { "abcd", "efghij", "klmnopqrs" }; + BOOST_FOREACH(const std::string& block, blocks) + { + strq.write(&block[0], block.length()); + } + std::vector longbuffer(30); + std::streamsize readlen(strq.read(&longbuffer[0], longbuffer.size())); + ensure_equals("read() multiple blocks", + readlen, blocks[0].length() + blocks[1].length() + blocks[2].length()); + ensure_equals("read() multiple blocks result", + std::string(longbuffer.begin(), longbuffer.begin() + readlen), + blocks[0] + blocks[1] + blocks[2]); + } + + template<> template<> + void object::test<6>() + { + set_test_name("split blocks"); + std::string blocks[] = { "abcdefghijklm", "nopqrstuvwxyz" }; + BOOST_FOREACH(const std::string& block, blocks) + { + strq.write(&block[0], block.length()); + } + strq.close(); + std::streamsize readlen(strq.read(&buffer[0], buffer.size())); + ensure_equals("read() 0", readlen, buffer.size()); + ensure_equals("read() 0 result", std::string(buffer.begin(), buffer.end()), "abcdefghij"); + readlen = strq.read(&buffer[0], buffer.size()); + ensure_equals("read() 1", readlen, buffer.size()); + ensure_equals("read() 1 result", std::string(buffer.begin(), buffer.end()), "klmnopqrst"); + readlen = strq.read(&buffer[0], buffer.size()); + ensure_equals("read() 2", readlen, 6); + ensure_equals("read() 2 result", + std::string(buffer.begin(), buffer.begin() + readlen), "uvwxyz"); + ensure_equals("read() 3", strq.read(&buffer[0], buffer.size()), -1); + } +} // namespace tut -- cgit v1.2.3 From 39a86eda8d6d810bd7f4dd6b96f022548a496ba1 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 12 Jan 2012 12:19:27 -0500 Subject: Add LLStreamQueue::size() and tests to exercise it. --- indra/llcommon/tests/llstreamqueue_test.cpp | 30 ++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llstreamqueue_test.cpp b/indra/llcommon/tests/llstreamqueue_test.cpp index e88c37d5be..050ad5c5bf 100644 --- a/indra/llcommon/tests/llstreamqueue_test.cpp +++ b/indra/llcommon/tests/llstreamqueue_test.cpp @@ -50,6 +50,8 @@ namespace tut { set_test_name("empty LLStreamQueue"); ensure_equals("brand-new LLStreamQueue isn't empty", + strq.size(), 0); + ensure_equals("brand-new LLStreamQueue returns data", strq.asSource().read(&buffer[0], buffer.size()), 0); strq.asSink().close(); ensure_equals("closed empty LLStreamQueue not at EOF", @@ -62,18 +64,22 @@ namespace tut set_test_name("one internal block, one buffer"); LLStreamQueue::Sink sink(strq.asSink()); ensure_equals("write(\"\")", sink.write("", 0), 0); - ensure_equals("0 write should leave LLStreamQueue empty", + ensure_equals("0 write should leave LLStreamQueue empty (size())", + strq.size(), 0); + ensure_equals("0 write should leave LLStreamQueue empty (peek())", strq.peek(&buffer[0], buffer.size()), 0); // The meaning of "atomic" is that it must be smaller than our buffer. std::string atomic("atomic"); ensure("test data exceeds buffer", atomic.length() < buffer.size()); ensure_equals(STRINGIZE("write(\"" << atomic << "\")"), sink.write(&atomic[0], atomic.length()), atomic.length()); + ensure_equals("size() after write()", strq.size(), atomic.length()); size_t peeklen(strq.peek(&buffer[0], buffer.size())); ensure_equals(STRINGIZE("peek(\"" << atomic << "\")"), peeklen, atomic.length()); ensure_equals(STRINGIZE("peek(\"" << atomic << "\") result"), std::string(buffer.begin(), buffer.begin() + peeklen), atomic); + ensure_equals("size() after peek()", strq.size(), atomic.length()); // peek() should not consume. Use a different buffer to prove it isn't // just leftover data from the first peek(). std::vector again(buffer.size()); @@ -90,6 +96,7 @@ namespace tut ensure_equals(STRINGIZE("read(\"" << atomic << "\") result"), std::string(third.begin(), third.begin() + readlen), atomic); ensure_equals("peek() after read()", strq.peek(&buffer[0], buffer.size()), 0); + ensure_equals("size() after read()", strq.size(), 0); } template<> template<> @@ -107,6 +114,7 @@ namespace tut std::string(buffer.begin(), buffer.begin() + peeklen), lovecraft); std::streamsize skip1(4); ensure_equals(STRINGIZE("skip(" << skip1 << ")"), strq.skip(skip1), skip1); + ensure_equals("size() after skip()", strq.size(), lovecraft.length() - skip1); size_t readlen(strq.read(&buffer[0], buffer.size())); ensure_equals(STRINGIZE("read(\"" << lovecraft.substr(skip1) << "\")"), readlen, lovecraft.length() - skip1); @@ -121,15 +129,20 @@ namespace tut { set_test_name("skip() multiple blocks"); std::string blocks[] = { "books of ", "H.P. ", "Lovecraft" }; - std::streamsize skip(blocks[0].length() + blocks[1].length() + 4); + std::streamsize total(blocks[0].length() + blocks[1].length() + blocks[2].length()); + std::streamsize leave(5); // len("craft") above + std::streamsize skip(total - leave); + std::streamsize written(0); BOOST_FOREACH(const std::string& block, blocks) { - strq.write(&block[0], block.length()); + written += strq.write(&block[0], block.length()); + ensure_equals("size() after write()", strq.size(), written); } std::streamsize skiplen(strq.skip(skip)); ensure_equals(STRINGIZE("skip(" << skip << ")"), skiplen, skip); + ensure_equals("size() after skip()", strq.size(), leave); size_t readlen(strq.read(&buffer[0], buffer.size())); - ensure_equals("read(\"craft\")", readlen, 5); + ensure_equals("read(\"craft\")", readlen, leave); ensure_equals("read(\"craft\") result", std::string(buffer.begin(), buffer.begin() + readlen), "craft"); } @@ -162,14 +175,21 @@ namespace tut strq.write(&block[0], block.length()); } strq.close(); + // We've already verified what strq.size() should be at this point; + // see above test named "skip() multiple blocks" + std::streamsize chksize(strq.size()); std::streamsize readlen(strq.read(&buffer[0], buffer.size())); ensure_equals("read() 0", readlen, buffer.size()); ensure_equals("read() 0 result", std::string(buffer.begin(), buffer.end()), "abcdefghij"); + chksize -= readlen; + ensure_equals("size() after read() 0", strq.size(), chksize); readlen = strq.read(&buffer[0], buffer.size()); ensure_equals("read() 1", readlen, buffer.size()); ensure_equals("read() 1 result", std::string(buffer.begin(), buffer.end()), "klmnopqrst"); + chksize -= readlen; + ensure_equals("size() after read() 1", strq.size(), chksize); readlen = strq.read(&buffer[0], buffer.size()); - ensure_equals("read() 2", readlen, 6); + ensure_equals("read() 2", readlen, chksize); ensure_equals("read() 2 result", std::string(buffer.begin(), buffer.begin() + readlen), "uvwxyz"); ensure_equals("read() 3", strq.read(&buffer[0], buffer.size()), -1); -- cgit v1.2.3 From b6a08ad007deb855ce4d428654279206853a3b99 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 13 Jan 2012 12:41:54 -0500 Subject: Extract APR and temp-fixture-file helper code to indra/test. Specifically: Introduce ManageAPR class in indra/test/manageapr.h. This is useful for a simple test program without lots of static constructors. Extract NamedTempFile from llsdserialize_test.cpp to indra/test/ namedtempfile.h. Refactor to use APR file operations rather than platform- dependent APIs. Use NamedTempFile for llprocesslauncher_test.cpp. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 73 ++----- indra/llcommon/tests/llsdserialize_test.cpp | 261 +----------------------- 2 files changed, 26 insertions(+), 308 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index 4d8f850d92..3935c64a94 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -18,14 +18,14 @@ #include #include // std headers -#include // external library headers #include "llapr.h" #include "apr_thread_proc.h" -#include "apr_file_io.h" #include // other Linden headers #include "../test/lltut.h" +#include "../test/manageapr.h" +#include "../test/namedtempfile.h" #include "stringize.h" #if defined(LL_WINDOWS) @@ -36,30 +36,8 @@ #include #endif -class APR -{ -public: - APR(): - pool(NULL) - { - apr_initialize(); - apr_pool_create(&pool, NULL); - } - - ~APR() - { - apr_terminate(); - } - - std::string strerror(apr_status_t rv) - { - char errbuf[256]; - apr_strerror(rv, errbuf, sizeof(errbuf)); - return errbuf; - } - - apr_pool_t *pool; -}; +// static instance of this manages APR init/cleanup +static ManageAPR manager; #define ensure_equals_(left, right) \ ensure_equals(STRINGIZE(#left << " != " << #right), (left), (right)) @@ -74,11 +52,11 @@ namespace tut { void aprchk_(const char* call, apr_status_t rv, apr_status_t expected=APR_SUCCESS) { - ensure_equals(STRINGIZE(call << " => " << rv << ": " << apr.strerror(rv)), + ensure_equals(STRINGIZE(call << " => " << rv << ": " << manager.strerror(rv)), rv, expected); } - APR apr; + LLAPRPool pool; }; typedef test_group llprocesslauncher_group; typedef llprocesslauncher_group::object object; @@ -186,19 +164,7 @@ namespace tut set_test_name("raw APR nonblocking I/O"); // Create a script file in a temporary place. - const char* tempdir = NULL; - aprchk(apr_temp_dir_get(&tempdir, apr.pool)); - - // Construct a temp filename template in that directory. - char *tempname = NULL; - aprchk(apr_filepath_merge(&tempname, tempdir, "testXXXXXX", 0, apr.pool)); - - // Create a temp file from that template. - apr_file_t* fp = NULL; - aprchk(apr_file_mktemp(&fp, tempname, APR_CREATE | APR_WRITE | APR_EXCL, apr.pool)); - - // Write it. - const char script[] = + NamedTempFile script("py", "import sys" EOL "import time" EOL EOL @@ -208,10 +174,7 @@ namespace tut "time.sleep(2)" EOL "print >>sys.stderr, 'stderr after wait'" EOL "sys.stderr.flush()" EOL - ; - apr_size_t len(sizeof(script)-1); - aprchk(apr_file_write(fp, script, &len)); - aprchk(apr_file_close(fp)); + ); // Arrange to track the history of our interaction with child: what we // fetched, which pipe it came from, how many tries it took before we @@ -221,30 +184,33 @@ namespace tut // Run the child process. apr_procattr_t *procattr = NULL; - aprchk(apr_procattr_create(&procattr, apr.pool)); + aprchk(apr_procattr_create(&procattr, pool.getAPRPool())); aprchk(apr_procattr_io_set(procattr, APR_CHILD_BLOCK, APR_CHILD_BLOCK, APR_CHILD_BLOCK)); aprchk(apr_procattr_cmdtype_set(procattr, APR_PROGRAM_PATH)); std::vector argv; apr_proc_t child; argv.push_back("python"); - argv.push_back(tempname); + // Have to have a named copy of this std::string so its c_str() value + // will persist. + std::string scriptname(script.getName()); + argv.push_back(scriptname.c_str()); argv.push_back(NULL); aprchk(apr_proc_create(&child, argv[0], &argv[0], NULL, // if we wanted to pass explicit environment procattr, - apr.pool)); + pool.getAPRPool())); // We do not want this child process to outlive our APR pool. On // destruction of the pool, forcibly kill the process. Tell APR to try // SIGTERM and wait 3 seconds. If that didn't work, use SIGKILL. - apr_pool_note_subprocess(apr.pool, &child, APR_KILL_AFTER_TIMEOUT); + apr_pool_note_subprocess(pool.getAPRPool(), &child, APR_KILL_AFTER_TIMEOUT); // arrange to call child_status_callback() WaitInfo wi(&child); - apr_proc_other_child_register(&child, child_status_callback, &wi, child.in, apr.pool); + apr_proc_other_child_register(&child, child_status_callback, &wi, child.in, pool.getAPRPool()); // TODO: // Stuff child.in until it (would) block to verify EWOULDBLOCK/EAGAIN. @@ -289,7 +255,7 @@ namespace tut } if (rv == EWOULDBLOCK || rv == EAGAIN) { -// std::cout << "(waiting; apr_file_gets(" << dfli->first << ") => " << rv << ": " << apr.strerror(rv) << ")\n"; +// std::cout << "(waiting; apr_file_gets(" << dfli->first << ") => " << rv << ": " << manager.strerror(rv) << ")\n"; ++history.back().tries; continue; } @@ -334,14 +300,11 @@ namespace tut std::cout << "child_status_callback(APR_OC_REASON_DEATH) wasn't called" << std::endl; wi.rv = apr_proc_wait(wi.child, &wi.rc, &wi.why, APR_NOWAIT); } -// std::cout << "child done: rv = " << rv << " (" << apr.strerror(rv) << "), why = " << why << ", rc = " << rc << '\n'; +// std::cout << "child done: rv = " << rv << " (" << manager.strerror(rv) << "), why = " << why << ", rc = " << rc << '\n'; aprchk_("apr_proc_wait(wi->child, &wi->rc, &wi->why, APR_NOWAIT)", wi.rv, APR_CHILD_DONE); ensure_equals_(wi.why, APR_PROC_EXIT); ensure_equals_(wi.rc, 0); - // Remove temp script file - aprchk(apr_file_remove(tempname, apr.pool)); - // Beyond merely executing all the above successfully, verify that we // obtained expected output -- and that we duly got control while // waiting, proving the non-blocking nature of these pipes. diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index 72322c3b72..4359e9afb9 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -43,38 +43,12 @@ typedef U32 uint32_t; #include "llprocesslauncher.h" #endif -#include - -/*==========================================================================*| -// Whoops, seems Linden's Boost package and the viewer are built with -// different settings of VC's /Zc:wchar_t switch! Using Boost.Filesystem -// pathname operations produces Windows link errors: -// unresolved external symbol "private: static class std::codecvt const * & __cdecl boost::filesystem3::path::wchar_t_codecvt_facet()" -// unresolved external symbol "void __cdecl boost::filesystem3::path_traits::convert()" -// See: -// http://boost.2283326.n4.nabble.com/filesystem-v3-unicode-and-std-codecvt-linker-error-td3455549.html -// which points to: -// http://msdn.microsoft.com/en-us/library/dh8che7s%28v=VS.100%29.aspx - -// As we're not trying to preserve compatibility with old Boost.Filesystem -// code, but rather writing brand-new code, use the newest available -// Filesystem API. -#define BOOST_FILESYSTEM_VERSION 3 -#include "boost/filesystem.hpp" -#include "boost/filesystem/v3/fstream.hpp" -|*==========================================================================*/ #include "boost/range.hpp" #include "boost/foreach.hpp" #include "boost/function.hpp" #include "boost/lambda/lambda.hpp" #include "boost/lambda/bind.hpp" namespace lambda = boost::lambda; -/*==========================================================================*| -// Aaaarrgh, Linden's Boost package doesn't even include Boost.Iostreams! -#include "boost/iostreams/stream.hpp" -#include "boost/iostreams/device/file_descriptor.hpp" -|*==========================================================================*/ #include "../llsd.h" #include "../llsdserialize.h" @@ -82,236 +56,17 @@ namespace lambda = boost::lambda; #include "../llformat.h" #include "../test/lltut.h" +#include "../test/manageapr.h" +#include "../test/namedtempfile.h" #include "stringize.h" +static ManageAPR manager; + std::vector string_to_vector(const std::string& str) { return std::vector(str.begin(), str.end()); } -#if ! LL_WINDOWS -// We want to call strerror_r(), but alarmingly, there are two different -// variants. The one that returns int always populates the passed buffer -// (except in case of error), whereas the other one always returns a valid -// char* but might or might not populate the passed buffer. How do we know -// which one we're getting? Define adapters for each and let the compiler -// select the applicable adapter. - -// strerror_r() returns char* -std::string message_from(int /*orig_errno*/, const char* /*buffer*/, const char* strerror_ret) -{ - return strerror_ret; -} - -// strerror_r() returns int -std::string message_from(int orig_errno, const char* buffer, int strerror_ret) -{ - if (strerror_ret == 0) - { - return buffer; - } - // Here strerror_r() has set errno. Since strerror_r() has already failed, - // seems like a poor bet to call it again to diagnose its own error... - int stre_errno = errno; - if (stre_errno == ERANGE) - { - return STRINGIZE("strerror_r() can't explain errno " << orig_errno - << " (buffer too small)"); - } - if (stre_errno == EINVAL) - { - return STRINGIZE("unknown errno " << orig_errno); - } - // Here we don't even understand the errno from strerror_r()! - return STRINGIZE("strerror_r() can't explain errno " << orig_errno - << " (error " << stre_errno << ')'); -} -#endif // ! LL_WINDOWS - -// boost::filesystem::temp_directory_path() isn't yet in Boost 1.45! :-( -std::string temp_directory_path() -{ -#if LL_WINDOWS - char buffer[4096]; - GetTempPathA(sizeof(buffer), buffer); - return buffer; - -#else // LL_DARWIN, LL_LINUX - static const char* vars[] = { "TMPDIR", "TMP", "TEMP", "TEMPDIR" }; - BOOST_FOREACH(const char* var, vars) - { - const char* found = getenv(var); - if (found) - return found; - } - return "/tmp"; -#endif // LL_DARWIN, LL_LINUX -} - -// Windows presents a kinda sorta compatibility layer. Code to the yucky -// Windows names because they're less likely than the Posix names to collide -// with any other names in this source. -#if LL_WINDOWS -#define _remove DeleteFileA -#else // ! LL_WINDOWS -#define _open open -#define _write write -#define _close close -#define _remove remove -#endif // ! LL_WINDOWS - -// Create a text file with specified content "somewhere in the -// filesystem," cleaning up when it goes out of scope. -class NamedTempFile -{ -public: - // Function that accepts an ostream ref and (presumably) writes stuff to - // it, e.g.: - // (lambda::_1 << "the value is " << 17 << '\n') - typedef boost::function Streamer; - - NamedTempFile(const std::string& ext, const std::string& content): - mPath(temp_directory_path()) - { - createFile(ext, lambda::_1 << content); - } - - // Disambiguate when passing string literal - NamedTempFile(const std::string& ext, const char* content): - mPath(temp_directory_path()) - { - createFile(ext, lambda::_1 << content); - } - - NamedTempFile(const std::string& ext, const Streamer& func): - mPath(temp_directory_path()) - { - createFile(ext, func); - } - - ~NamedTempFile() - { - _remove(mPath.c_str()); - } - - std::string getName() const { return mPath; } - -private: - void createFile(const std::string& ext, const Streamer& func) - { - // Silly maybe, but use 'ext' as the name prefix. Strip off a leading - // '.' if present. - int pfx_offset = ((! ext.empty()) && ext[0] == '.')? 1 : 0; - -#if ! LL_WINDOWS - // Make sure mPath ends with a directory separator, if it doesn't already. - if (mPath.empty() || - ! (mPath[mPath.length() - 1] == '\\' || mPath[mPath.length() - 1] == '/')) - { - mPath.append("/"); - } - - // mkstemp() accepts and modifies a char* template string. Generate - // the template string, then copy to modifiable storage. - // mkstemp() requires its template string to end in six X's. - mPath += ext.substr(pfx_offset) + "XXXXXX"; - // Copy to vector - std::vector pathtemplate(mPath.begin(), mPath.end()); - // append a nul byte for classic-C semantics - pathtemplate.push_back('\0'); - // std::vector promises that a pointer to the 0th element is the same - // as a pointer to a contiguous classic-C array - int fd(mkstemp(&pathtemplate[0])); - if (fd == -1) - { - // The documented errno values (http://linux.die.net/man/3/mkstemp) - // are used in a somewhat unusual way, so provide context-specific - // errors. - if (errno == EEXIST) - { - LL_ERRS("NamedTempFile") << "mkstemp(\"" << mPath - << "\") could not create unique file " << LL_ENDL; - } - if (errno == EINVAL) - { - LL_ERRS("NamedTempFile") << "bad mkstemp() file path template '" - << mPath << "'" << LL_ENDL; - } - // Shrug, something else - int mkst_errno = errno; - char buffer[256]; - LL_ERRS("NamedTempFile") << "mkstemp(\"" << mPath << "\") failed: " - << message_from(mkst_errno, buffer, - strerror_r(mkst_errno, buffer, sizeof(buffer))) - << LL_ENDL; - } - // mkstemp() seems to have worked! Capture the modified filename. - // Avoid the nul byte we appended. - mPath.assign(pathtemplate.begin(), (pathtemplate.end()-1)); - -/*==========================================================================*| - // Define an ostream on the open fd. Tell it to close fd on destruction. - boost::iostreams::stream - out(fd, boost::iostreams::close_handle); -|*==========================================================================*/ - - // Write desired content. - std::ostringstream out; - // Stream stuff to it. - func(out); - - std::string data(out.str()); - int written(_write(fd, data.c_str(), data.length())); - int closed(_close(fd)); - llassert_always(written == data.length() && closed == 0); - -#else // LL_WINDOWS - // GetTempFileName() is documented to require a MAX_PATH buffer. - char tempname[MAX_PATH]; - // Use 'ext' as filename prefix, but skip leading '.' if any. - // The 0 param is very important: requests iterating until we get a - // unique name. - if (0 == GetTempFileNameA(mPath.c_str(), ext.c_str() + pfx_offset, 0, tempname)) - { - // I always have to look up this call... :-P - LPSTR msgptr; - FormatMessageA( - FORMAT_MESSAGE_ALLOCATE_BUFFER | - FORMAT_MESSAGE_FROM_SYSTEM | - FORMAT_MESSAGE_IGNORE_INSERTS, - NULL, - GetLastError(), - MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), - LPSTR(&msgptr), // have to cast (char**) to (char*) - 0, NULL ); - LL_ERRS("NamedTempFile") << "GetTempFileName(\"" << mPath << "\", \"" - << (ext.c_str() + pfx_offset) << "\") failed: " - << msgptr << LL_ENDL; - LocalFree(msgptr); - } - // GetTempFileName() appears to have worked! Capture the actual - // filename. - mPath = tempname; - // Open the file and stream content to it. Destructor will close. - std::ofstream out(tempname); - func(out); - -#endif // LL_WINDOWS - } - - void peep() - { - std::cout << "File '" << mPath << "' contains:\n"; - std::ifstream reader(mPath.c_str()); - std::string line; - while (std::getline(reader, line)) - std::cout << line << '\n'; - std::cout << "---\n"; - } - - std::string mPath; -}; - namespace tut { struct sd_xml_data @@ -1783,7 +1538,7 @@ namespace tut const char* PYTHON(getenv("PYTHON")); ensure("Set $PYTHON to the Python interpreter", PYTHON); - NamedTempFile scriptfile(".py", script); + NamedTempFile scriptfile("py", script); #if LL_WINDOWS std::string q("\""); @@ -1888,12 +1643,12 @@ namespace tut " else:\n" " assert False, 'Too many data items'\n"; - // Create a something.llsd file containing 'data' serialized to + // Create an llsdXXXXXX file containing 'data' serialized to // notation. It's important to separate with newlines because Python's // llsd module doesn't support parsing from a file stream, only from a // string, so we have to know how much of the file to read into a // string. - NamedTempFile file(".llsd", + NamedTempFile file("llsd", // NamedTempFile's boost::function constructor // takes a callable. To this callable it passes the // std::ostream with which it's writing the @@ -1926,7 +1681,7 @@ namespace tut // Create an empty data file. This is just a placeholder for our // script to write into. Create it to establish a unique name that // we know. - NamedTempFile file(".llsd", ""); + NamedTempFile file("llsd", ""); python("write Python notation", lambda::_1 << -- cgit v1.2.3 From 74fbd31813494fe120211fbdad3ed6da9c2d5d8b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 17 Jan 2012 19:03:17 -0500 Subject: Add first couple of LLProcessLauncher tests. Run INTEGRATION_TEST_llprocesslauncher using setpython.py so we can find the Python interpreter of interest. Introduce python() function to run a Python script specified using NamedTempFile conventions. Introduce a convention by which we can read output from a Python script using only the limited pre-January-2012 LLProcessLauncher API. Introduce python_out() function to leverage that convention. Exercise a couple of LLProcessLauncher methods using all the above. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 146 ++++++++++++++++++++++++ 1 file changed, 146 insertions(+) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index 3935c64a94..aebd280c2e 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -18,10 +18,14 @@ #include #include // std headers +#include // external library headers #include "llapr.h" #include "apr_thread_proc.h" #include +#include +#include +#include // other Linden headers #include "../test/lltut.h" #include "../test/manageapr.h" @@ -36,6 +40,8 @@ #include #endif +namespace lambda = boost::lambda; + // static instance of this manages APR init/cleanup static ManageAPR manager; @@ -56,6 +62,116 @@ namespace tut rv, expected); } + /** + * Run a Python script using LLProcessLauncher. + * @param desc Arbitrary description for error messages + * @param script Python script, any form acceptable to NamedTempFile, + * typically either a std::string or an expression of the form + * (lambda::_1 << "script content with " << variable_data) + * @param arg If specified, will be passed to script as its + * sys.argv[1] + * @param tweak "Do something" to LLProcessLauncher object before + * calling its launch() method. This program is to test + * LLProcessLauncher, but many such tests are "just like" this + * python() function but for one or two extra method calls before + * launch(). This avoids us having to clone & edit this function for + * such tests. + */ + template + void python(const std::string& desc, const CONTENT& script, const std::string& arg="", + const boost::function tweak=lambda::_1) + { + const char* PYTHON(getenv("PYTHON")); + ensure("Set $PYTHON to the Python interpreter", PYTHON); + + NamedTempFile scriptfile("py", script); + LLProcessLauncher py; + py.setExecutable(PYTHON); + py.addArgument(scriptfile.getName()); + if (! arg.empty()) + { + py.addArgument(arg); + } + tweak(py); + ensure_equals(STRINGIZE("Couldn't launch " << desc << " script"), py.launch(), 0); + // One of the irritating things about LLProcessLauncher is that + // there's no API to wait for the child to terminate -- but given + // its use in our graphics-intensive interactive viewer, it's + // understandable. + while (py.isRunning()) + { + sleep(1); + } + } + + /** + * Run a Python script using LLProcessLauncher, expecting that it will + * write to the file passed as its sys.argv[1]. Retrieve that output. + * + * Until January 2012, LLProcessLauncher provided distressingly few + * mechanisms for a child process to communicate back to its caller -- + * not even its return code. We've introduced a convention by which we + * create an empty temp file, pass the name of that file to our child + * as sys.argv[1] and expect the script to write its output to that + * file. This function implements the C++ (parent process) side of + * that convention. + * + * @param desc as for python() + * @param script as for python() + * @param tweak as for python() + */ + template + std::string python_out(const std::string& desc, const CONTENT& script, + const boost::function tweak=lambda::_1) + { + NamedTempFile out("out", ""); // placeholder + // pass name of this temporary file to the script + python(desc, script, out.getName(), tweak); + // assuming the script wrote a line to that file, read it + std::string output; + { + std::ifstream inf(out.getName().c_str()); + ensure(STRINGIZE("No output from " << desc << " script"), + std::getline(inf, output)); + std::string more; + while (std::getline(inf, more)) + { + output += '\n' + more; + } + } // important to close inf BEFORE removing NamedTempFile + return output; + } + + class NamedTempDir + { + public: + // Use python() function to create a temp directory: I've found + // nothing in either Boost.Filesystem or APR quite like Python's + // tempfile.mkdtemp(). + // Special extra bonus: on Mac, mkdtemp() reports a pathname + // starting with /var/folders/something, whereas that's really a + // symlink to /private/var/folders/something. Have to use + // realpath() to compare properly. + NamedTempDir(llprocesslauncher_data* ths): + mThis(ths), + mPath(ths->python_out("mkdtemp()", + "import os.path, sys, tempfile\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write(os.path.realpath(tempfile.mkdtemp()))\n")) + {} + + ~NamedTempDir() + { + mThis->aprchk(apr_dir_remove(mPath.c_str(), gAPRPoolp)); + } + + std::string getName() const { return mPath; } + + private: + llprocesslauncher_data* mThis; + std::string mPath; + }; + LLAPRPool pool; }; typedef test_group llprocesslauncher_group; @@ -349,4 +465,34 @@ namespace tut throw; } } + + template<> template<> + void object::test<2>() + { + set_test_name("set/getExecutable()"); + LLProcessLauncher child; + child.setExecutable("nonsense string"); + ensure_equals("setExecutable() 0", child.getExecutable(), "nonsense string"); + child.setExecutable("python"); + ensure_equals("setExecutable() 1", child.getExecutable(), "python"); + } + + template<> template<> + void object::test<3>() + { + set_test_name("setWorkingDirectory()"); + // We want to test setWorkingDirectory(). But what directory is + // guaranteed to exist on every machine, under every OS? Have to + // create one. + NamedTempDir tempdir(this); + std::string cwd(python_out("getcwd()", + "import os, sys\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write(os.getcwd())\n", + // Before LLProcessLauncher::launch(), call setWorkingDirectory() + lambda::bind(&LLProcessLauncher::setWorkingDirectory, + lambda::_1, + tempdir.getName()))); + ensure_equals("os.getcwd()", cwd, tempdir.getName()); + } } // namespace tut -- cgit v1.2.3 From 2ae9f921f2e1d6bd10e4c334a19312761a914046 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 17 Jan 2012 20:44:26 -0500 Subject: Refactor llprocesslauncher_test.cpp for better code reuse. Instead of free python() and python_out() functions containing a local temporary LLProcessLauncher instance, with a 'tweak' callback param to "do stuff" to that inaccessible object, change to a PythonProcessLauncher class that sets up a (public) LLProcessLauncher member, then allows you to run() or run() and then readfile() the output. Now you can construct an instance and tweak to your heart's content -- without funky callback syntax -- before running the script. Move all such helpers from TUT fixture struct to namespace scope. While fixture-struct methods can freely call one another, introducing a nested class gets awkward: constructor must explicitly require and bind a fixture-struct pointer or reference. Namespace scope solves this. (Truthfully, I only put them in the fixture struct originally because I thought it necessary for calling ensure() et al. But ensure() and friends are free functions; need only qualify them with tut:: namespace.) --- indra/llcommon/tests/llprocesslauncher_test.cpp | 282 +++++++++++++----------- 1 file changed, 155 insertions(+), 127 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index aebd280c2e..310271e465 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -12,7 +12,6 @@ // Precompiled header #include "linden_common.h" // associated header -#define WIN32_LEAN_AND_MEAN #include "llprocesslauncher.h" // STL headers #include @@ -24,8 +23,8 @@ #include "apr_thread_proc.h" #include #include -#include -#include +//#include +//#include // other Linden headers #include "../test/lltut.h" #include "../test/manageapr.h" @@ -40,138 +39,169 @@ #include #endif -namespace lambda = boost::lambda; +//namespace lambda = boost::lambda; // static instance of this manages APR init/cleanup static ManageAPR manager; +/***************************************************************************** +* Helpers +*****************************************************************************/ + #define ensure_equals_(left, right) \ ensure_equals(STRINGIZE(#left << " != " << #right), (left), (right)) + #define aprchk(expr) aprchk_(#expr, (expr)) +static void aprchk_(const char* call, apr_status_t rv, apr_status_t expected=APR_SUCCESS) +{ + tut::ensure_equals(STRINGIZE(call << " => " << rv << ": " << manager.strerror(rv)), + rv, expected); +} -/***************************************************************************** -* TUT -*****************************************************************************/ -namespace tut +/** + * Read specified file using std::getline(). It is assumed to be an error if + * the file is empty: don't use this function if that's an acceptable case. + * Last line will not end with '\n'; this is to facilitate the usual case of + * string compares with a single line of output. + * @param pathname The file to read. + * @param desc Optional description of the file for error message; + * defaults to "in " + */ +static std::string readfile(const std::string& pathname, const std::string& desc="") { - struct llprocesslauncher_data + std::string use_desc(desc); + if (use_desc.empty()) { - void aprchk_(const char* call, apr_status_t rv, apr_status_t expected=APR_SUCCESS) - { - ensure_equals(STRINGIZE(call << " => " << rv << ": " << manager.strerror(rv)), - rv, expected); - } + use_desc = STRINGIZE("in " << pathname); + } + std::ifstream inf(pathname.c_str()); + std::string output; + tut::ensure(STRINGIZE("No output " << use_desc), std::getline(inf, output)); + std::string more; + while (std::getline(inf, more)) + { + output += '\n' + more; + } + return output; +} - /** - * Run a Python script using LLProcessLauncher. - * @param desc Arbitrary description for error messages - * @param script Python script, any form acceptable to NamedTempFile, - * typically either a std::string or an expression of the form - * (lambda::_1 << "script content with " << variable_data) - * @param arg If specified, will be passed to script as its - * sys.argv[1] - * @param tweak "Do something" to LLProcessLauncher object before - * calling its launch() method. This program is to test - * LLProcessLauncher, but many such tests are "just like" this - * python() function but for one or two extra method calls before - * launch(). This avoids us having to clone & edit this function for - * such tests. - */ - template - void python(const std::string& desc, const CONTENT& script, const std::string& arg="", - const boost::function tweak=lambda::_1) - { - const char* PYTHON(getenv("PYTHON")); - ensure("Set $PYTHON to the Python interpreter", PYTHON); - - NamedTempFile scriptfile("py", script); - LLProcessLauncher py; - py.setExecutable(PYTHON); - py.addArgument(scriptfile.getName()); - if (! arg.empty()) - { - py.addArgument(arg); - } - tweak(py); - ensure_equals(STRINGIZE("Couldn't launch " << desc << " script"), py.launch(), 0); - // One of the irritating things about LLProcessLauncher is that - // there's no API to wait for the child to terminate -- but given - // its use in our graphics-intensive interactive viewer, it's - // understandable. - while (py.isRunning()) - { - sleep(1); - } - } +/** + * Construct an LLProcessLauncher to run a Python script. + */ +struct PythonProcessLauncher +{ + /** + * @param desc Arbitrary description for error messages + * @param script Python script, any form acceptable to NamedTempFile, + * typically either a std::string or an expression of the form + * (lambda::_1 << "script content with " << variable_data) + */ + template + PythonProcessLauncher(const std::string& desc, const CONTENT& script): + mDesc(desc), + mScript("py", script) + { + const char* PYTHON(getenv("PYTHON")); + tut::ensure("Set $PYTHON to the Python interpreter", PYTHON); + + mPy.setExecutable(PYTHON); + mPy.addArgument(mScript.getName()); + } - /** - * Run a Python script using LLProcessLauncher, expecting that it will - * write to the file passed as its sys.argv[1]. Retrieve that output. - * - * Until January 2012, LLProcessLauncher provided distressingly few - * mechanisms for a child process to communicate back to its caller -- - * not even its return code. We've introduced a convention by which we - * create an empty temp file, pass the name of that file to our child - * as sys.argv[1] and expect the script to write its output to that - * file. This function implements the C++ (parent process) side of - * that convention. - * - * @param desc as for python() - * @param script as for python() - * @param tweak as for python() - */ - template - std::string python_out(const std::string& desc, const CONTENT& script, - const boost::function tweak=lambda::_1) + /// Run Python script and wait for it to complete. + void run() + { + tut::ensure_equals(STRINGIZE("Couldn't launch " << mDesc << " script"), + mPy.launch(), 0); + // One of the irritating things about LLProcessLauncher is that + // there's no API to wait for the child to terminate -- but given + // its use in our graphics-intensive interactive viewer, it's + // understandable. + while (mPy.isRunning()) { - NamedTempFile out("out", ""); // placeholder - // pass name of this temporary file to the script - python(desc, script, out.getName(), tweak); - // assuming the script wrote a line to that file, read it - std::string output; - { - std::ifstream inf(out.getName().c_str()); - ensure(STRINGIZE("No output from " << desc << " script"), - std::getline(inf, output)); - std::string more; - while (std::getline(inf, more)) - { - output += '\n' + more; - } - } // important to close inf BEFORE removing NamedTempFile - return output; + sleep(1); } + } - class NamedTempDir - { - public: - // Use python() function to create a temp directory: I've found - // nothing in either Boost.Filesystem or APR quite like Python's - // tempfile.mkdtemp(). - // Special extra bonus: on Mac, mkdtemp() reports a pathname - // starting with /var/folders/something, whereas that's really a - // symlink to /private/var/folders/something. Have to use - // realpath() to compare properly. - NamedTempDir(llprocesslauncher_data* ths): - mThis(ths), - mPath(ths->python_out("mkdtemp()", - "import os.path, sys, tempfile\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write(os.path.realpath(tempfile.mkdtemp()))\n")) - {} - - ~NamedTempDir() - { - mThis->aprchk(apr_dir_remove(mPath.c_str(), gAPRPoolp)); - } + /** + * Run a Python script using LLProcessLauncher, expecting that it will + * write to the file passed as its sys.argv[1]. Retrieve that output. + * + * Until January 2012, LLProcessLauncher provided distressingly few + * mechanisms for a child process to communicate back to its caller -- + * not even its return code. We've introduced a convention by which we + * create an empty temp file, pass the name of that file to our child + * as sys.argv[1] and expect the script to write its output to that + * file. This function implements the C++ (parent process) side of + * that convention. + */ + std::string run_read() + { + NamedTempFile out("out", ""); // placeholder + // pass name of this temporary file to the script + mPy.addArgument(out.getName()); + run(); + // assuming the script wrote to that file, read it + return readfile(out.getName(), STRINGIZE("from " << mDesc << " script")); + } + + LLProcessLauncher mPy; + std::string mDesc; + NamedTempFile mScript; +}; + +/// convenience function for PythonProcessLauncher::run() +template +static void python(const std::string& desc, const CONTENT& script) +{ + PythonProcessLauncher py(desc, script); + py.run(); +} + +/// convenience function for PythonProcessLauncher::run_read() +template +static std::string python_out(const std::string& desc, const CONTENT& script) +{ + PythonProcessLauncher py(desc, script); + return py.run_read(); +} + +/// Create a temporary directory and clean it up later. +class NamedTempDir: public boost::noncopyable +{ +public: + // Use python() function to create a temp directory: I've found + // nothing in either Boost.Filesystem or APR quite like Python's + // tempfile.mkdtemp(). + // Special extra bonus: on Mac, mkdtemp() reports a pathname + // starting with /var/folders/something, whereas that's really a + // symlink to /private/var/folders/something. Have to use + // realpath() to compare properly. + NamedTempDir(): + mPath(python_out("mkdtemp()", + "import os.path, sys, tempfile\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write(os.path.realpath(tempfile.mkdtemp()))\n")) + {} + + ~NamedTempDir() + { + aprchk(apr_dir_remove(mPath.c_str(), gAPRPoolp)); + } - std::string getName() const { return mPath; } + std::string getName() const { return mPath; } - private: - llprocesslauncher_data* mThis; - std::string mPath; - }; +private: + std::string mPath; +}; +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + struct llprocesslauncher_data + { LLAPRPool pool; }; typedef test_group llprocesslauncher_group; @@ -484,15 +514,13 @@ namespace tut // We want to test setWorkingDirectory(). But what directory is // guaranteed to exist on every machine, under every OS? Have to // create one. - NamedTempDir tempdir(this); - std::string cwd(python_out("getcwd()", - "import os, sys\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write(os.getcwd())\n", - // Before LLProcessLauncher::launch(), call setWorkingDirectory() - lambda::bind(&LLProcessLauncher::setWorkingDirectory, - lambda::_1, - tempdir.getName()))); - ensure_equals("os.getcwd()", cwd, tempdir.getName()); + NamedTempDir tempdir; + PythonProcessLauncher py("getcwd()", + "import os, sys\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write(os.getcwd())\n"); + // Before running, call setWorkingDirectory() + py.mPy.setWorkingDirectory(tempdir.getName()); + ensure_equals("os.getcwd()", py.run_read(), tempdir.getName()); } } // namespace tut -- cgit v1.2.3 From 4bfd84d3be8d33bc6eb0dab22d2b3034de0800c9 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 17 Jan 2012 21:40:41 -0500 Subject: Add tests for child-process args management and for kill() method. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 80 ++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index 310271e465..4f6a6ed922 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -23,6 +23,8 @@ #include "apr_thread_proc.h" #include #include +#include +#include //#include //#include // other Linden headers @@ -513,7 +515,7 @@ namespace tut set_test_name("setWorkingDirectory()"); // We want to test setWorkingDirectory(). But what directory is // guaranteed to exist on every machine, under every OS? Have to - // create one. + // create one. Naturally, ensure we clean it up when done. NamedTempDir tempdir; PythonProcessLauncher py("getcwd()", "import os, sys\n" @@ -523,4 +525,80 @@ namespace tut py.mPy.setWorkingDirectory(tempdir.getName()); ensure_equals("os.getcwd()", py.run_read(), tempdir.getName()); } + + template<> template<> + void object::test<4>() + { + set_test_name("clearArguments()"); + PythonProcessLauncher py("args", + "import sys\n" + // note nonstandard output-file arg! + "with open(sys.argv[3], 'w') as f:\n" + " for arg in sys.argv[1:]:\n" + " print >>f, arg\n"); + // We expect that PythonProcessLauncher has already called + // addArgument() with the name of its own NamedTempFile. But let's + // change it up. + py.mPy.clearArguments(); + // re-add script pathname + py.mPy.addArgument(py.mScript.getName()); // sys.argv[0] + py.mPy.addArgument("first arg"); // sys.argv[1] + py.mPy.addArgument("second arg"); // sys.argv[2] + // run_read() calls addArgument() one more time, hence [3] + std::string output(py.run_read()); + boost::split_iterator + li(output, boost::first_finder("\n")), lend; + ensure("didn't get first arg", li != lend); + std::string arg(li->begin(), li->end()); + ensure_equals(arg, "first arg"); + ++li; + ensure("didn't get second arg", li != lend); + arg.assign(li->begin(), li->end()); + ensure_equals(arg, "second arg"); + ++li; + ensure("didn't get output filename?!", li != lend); + arg.assign(li->begin(), li->end()); + ensure("output filename empty?!", ! arg.empty()); + ++li; + ensure("too many args", li == lend); + } + + template<> template<> + void object::test<5>() + { + set_test_name("kill()"); + PythonProcessLauncher py("kill()", + "import sys, time\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('ok')\n" + "# now sleep; expect caller to kill\n" + "time.sleep(120)\n" + "# if caller hasn't managed to kill by now, bad\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('bad')\n"); + NamedTempFile out("out", "not started"); + py.mPy.addArgument(out.getName()); + ensure_equals("couldn't launch kill() script", py.mPy.launch(), 0); + // Wait for the script to wake up and do its first write + int i = 0, timeout = 60; + for ( ; i < timeout; ++i) + { + sleep(1); + if (readfile(out.getName(), "from kill() script") == "ok") + break; + } + // If we broke this loop because of the counter, something's wrong + ensure("script never started", i < timeout); + // script has performed its first write and should now be sleeping. + py.mPy.kill(); + // wait for the script to terminate... one way or another. + while (py.mPy.isRunning()) + { + sleep(1); + } + // If kill() failed, the script would have woken up on its own and + // overwritten the file with 'bad'. But if kill() succeeded, it should + // not have had that chance. + ensure_equals("kill() script output", readfile(out.getName()), "ok"); + } } // namespace tut -- cgit v1.2.3 From ff4addd1b427344c6064734bdb59952e78f759fd Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 18 Jan 2012 01:10:52 -0500 Subject: Add tests for implicit-kill-on-destroy, also orphan() method. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 110 +++++++++++++++++++++++- 1 file changed, 109 insertions(+), 1 deletion(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index 4f6a6ed922..7c0f0eaa84 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -566,7 +566,7 @@ namespace tut template<> template<> void object::test<5>() { - set_test_name("kill()"); + set_test_name("explicit kill()"); PythonProcessLauncher py("kill()", "import sys, time\n" "with open(sys.argv[1], 'w') as f:\n" @@ -601,4 +601,112 @@ namespace tut // not have had that chance. ensure_equals("kill() script output", readfile(out.getName()), "ok"); } + + template<> template<> + void object::test<6>() + { + set_test_name("implicit kill()"); + NamedTempFile out("out", "not started"); + LLProcessLauncher::ll_pid_t pid(0); + { + PythonProcessLauncher py("kill()", + "import sys, time\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('ok')\n" + "# now sleep; expect caller to kill\n" + "time.sleep(120)\n" + "# if caller hasn't managed to kill by now, bad\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('bad')\n"); + py.mPy.addArgument(out.getName()); + ensure_equals("couldn't launch kill() script", py.mPy.launch(), 0); + // Capture ll_pid_t for later + pid = py.mPy.getProcessID(); + // Wait for the script to wake up and do its first write + int i = 0, timeout = 60; + for ( ; i < timeout; ++i) + { + sleep(1); + if (readfile(out.getName(), "from kill() script") == "ok") + break; + } + // If we broke this loop because of the counter, something's wrong + ensure("script never started", i < timeout); + // Script has performed its first write and should now be sleeping. + // Destroy the LLProcessLauncher, which should kill the child. + } + // wait for the script to terminate... one way or another. + while (LLProcessLauncher::isRunning(pid)) + { + sleep(1); + } + // If kill() failed, the script would have woken up on its own and + // overwritten the file with 'bad'. But if kill() succeeded, it should + // not have had that chance. + ensure_equals("kill() script output", readfile(out.getName()), "ok"); + } + + template<> template<> + void object::test<7>() + { + set_test_name("orphan()"); + NamedTempFile from("from", "not started"); + NamedTempFile to("to", ""); + LLProcessLauncher::ll_pid_t pid(0); + { + PythonProcessLauncher py("orphan()", + "import sys, time\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('ok')\n" + "# wait for 'go' from test program\n" + "for i in xrange(60):\n" + " time.sleep(1)\n" + " with open(sys.argv[2]) as f:\n" + " go = f.read()\n" + " if go == 'go':\n" + " break\n" + "else:\n" + " with open(sys.argv[1], 'w') as f:\n" + " f.write('never saw go')\n" + " sys.exit(1)\n" + "# okay, saw 'go', write 'ack'\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('ack')\n"); + py.mPy.addArgument(from.getName()); + py.mPy.addArgument(to.getName()); + ensure_equals("couldn't launch kill() script", py.mPy.launch(), 0); + // Capture ll_pid_t for later + pid = py.mPy.getProcessID(); + // Wait for the script to wake up and do its first write + int i = 0, timeout = 60; + for ( ; i < timeout; ++i) + { + sleep(1); + if (readfile(from.getName(), "from orphan() script") == "ok") + break; + } + // If we broke this loop because of the counter, something's wrong + ensure("script never started", i < timeout); + // Script has performed its first write and should now be waiting + // for us. Orphan it. + py.mPy.orphan(); + // Now destroy the LLProcessLauncher, which should NOT kill the child! + } + // If the destructor killed the child anyway, give it time to die + sleep(2); + // How do we know it's not terminated? By making it respond to + // a specific stimulus in a specific way. + { + std::ofstream outf(to.getName().c_str()); + outf << "go"; + } // flush and close. + // now wait for the script to terminate... one way or another. + while (LLProcessLauncher::isRunning(pid)) + { + sleep(1); + } + // If the LLProcessLauncher destructor implicitly called kill(), the + // script could not have written 'ack' as we expect. + ensure_equals("orphan() script output", readfile(from.getName()), "ack"); + } } // namespace tut -- cgit v1.2.3 From 1ed5bb3adaea0b4fee1e471575459039df8ced2f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 18 Jan 2012 10:56:13 -0500 Subject: Make embedded Python scripts compatible with Python 2.5 *SIGH* Apparently our TeamCity build machines are still not up to Python 2.6. --- indra/llcommon/tests/llprocesslauncher_test.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp index 7c0f0eaa84..057f83631e 100644 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ b/indra/llcommon/tests/llprocesslauncher_test.cpp @@ -181,6 +181,7 @@ public: // realpath() to compare properly. NamedTempDir(): mPath(python_out("mkdtemp()", + "from __future__ import with_statement\n" "import os.path, sys, tempfile\n" "with open(sys.argv[1], 'w') as f:\n" " f.write(os.path.realpath(tempfile.mkdtemp()))\n")) @@ -518,6 +519,7 @@ namespace tut // create one. Naturally, ensure we clean it up when done. NamedTempDir tempdir; PythonProcessLauncher py("getcwd()", + "from __future__ import with_statement\n" "import os, sys\n" "with open(sys.argv[1], 'w') as f:\n" " f.write(os.getcwd())\n"); @@ -531,6 +533,7 @@ namespace tut { set_test_name("clearArguments()"); PythonProcessLauncher py("args", + "from __future__ import with_statement\n" "import sys\n" // note nonstandard output-file arg! "with open(sys.argv[3], 'w') as f:\n" @@ -568,6 +571,7 @@ namespace tut { set_test_name("explicit kill()"); PythonProcessLauncher py("kill()", + "from __future__ import with_statement\n" "import sys, time\n" "with open(sys.argv[1], 'w') as f:\n" " f.write('ok')\n" @@ -610,6 +614,7 @@ namespace tut LLProcessLauncher::ll_pid_t pid(0); { PythonProcessLauncher py("kill()", + "from __future__ import with_statement\n" "import sys, time\n" "with open(sys.argv[1], 'w') as f:\n" " f.write('ok')\n" @@ -655,6 +660,7 @@ namespace tut LLProcessLauncher::ll_pid_t pid(0); { PythonProcessLauncher py("orphan()", + "from __future__ import with_statement\n" "import sys, time\n" "with open(sys.argv[1], 'w') as f:\n" " f.write('ok')\n" -- 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/tests/llprocess_test.cpp | 706 +++++++++++++++++++++++ indra/llcommon/tests/llprocesslauncher_test.cpp | 718 ------------------------ indra/llcommon/tests/llsdserialize_test.cpp | 13 +- 3 files changed, 713 insertions(+), 724 deletions(-) create mode 100644 indra/llcommon/tests/llprocess_test.cpp delete mode 100644 indra/llcommon/tests/llprocesslauncher_test.cpp (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp new file mode 100644 index 0000000000..55e22abd81 --- /dev/null +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -0,0 +1,706 @@ +/** + * @file llprocess_test.cpp + * @author Nat Goodspeed + * @date 2011-12-19 + * @brief Test for llprocess. + * + * $LicenseInfo:firstyear=2011&license=viewerlgpl$ + * Copyright (c) 2011, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llprocess.h" +// STL headers +#include +#include +// std headers +#include +// external library headers +#include "llapr.h" +#include "apr_thread_proc.h" +#include +#include +#include +#include +//#include +//#include +// other Linden headers +#include "../test/lltut.h" +#include "../test/manageapr.h" +#include "../test/namedtempfile.h" +#include "stringize.h" +#include "llsdutil.h" + +#if defined(LL_WINDOWS) +#define sleep(secs) _sleep((secs) * 1000) +#define EOL "\r\n" +#else +#define EOL "\n" +#include +#endif + +//namespace lambda = boost::lambda; + +// static instance of this manages APR init/cleanup +static ManageAPR manager; + +/***************************************************************************** +* Helpers +*****************************************************************************/ + +#define ensure_equals_(left, right) \ + ensure_equals(STRINGIZE(#left << " != " << #right), (left), (right)) + +#define aprchk(expr) aprchk_(#expr, (expr)) +static void aprchk_(const char* call, apr_status_t rv, apr_status_t expected=APR_SUCCESS) +{ + tut::ensure_equals(STRINGIZE(call << " => " << rv << ": " << manager.strerror(rv)), + rv, expected); +} + +/** + * Read specified file using std::getline(). It is assumed to be an error if + * the file is empty: don't use this function if that's an acceptable case. + * Last line will not end with '\n'; this is to facilitate the usual case of + * string compares with a single line of output. + * @param pathname The file to read. + * @param desc Optional description of the file for error message; + * defaults to "in " + */ +static std::string readfile(const std::string& pathname, const std::string& desc="") +{ + std::string use_desc(desc); + if (use_desc.empty()) + { + use_desc = STRINGIZE("in " << pathname); + } + std::ifstream inf(pathname.c_str()); + std::string output; + tut::ensure(STRINGIZE("No output " << use_desc), std::getline(inf, output)); + std::string more; + while (std::getline(inf, more)) + { + output += '\n' + more; + } + return output; +} + +/** + * Construct an LLProcess to run a Python script. + */ +struct PythonProcessLauncher +{ + /** + * @param desc Arbitrary description for error messages + * @param script Python script, any form acceptable to NamedTempFile, + * typically either a std::string or an expression of the form + * (lambda::_1 << "script content with " << variable_data) + */ + template + PythonProcessLauncher(const std::string& desc, const CONTENT& script): + mDesc(desc), + mScript("py", script) + { + const char* PYTHON(getenv("PYTHON")); + tut::ensure("Set $PYTHON to the Python interpreter", PYTHON); + + mParams["executable"] = PYTHON; + mParams["args"].append(mScript.getName()); + } + + /// Run Python script and wait for it to complete. + void run() + { + mPy = LLProcess::create(mParams); + tut::ensure(STRINGIZE("Couldn't launch " << mDesc << " script"), mPy); + // One of the irritating things about LLProcess is that + // there's no API to wait for the child to terminate -- but given + // its use in our graphics-intensive interactive viewer, it's + // understandable. + while (mPy->isRunning()) + { + sleep(1); + } + } + + /** + * Run a Python script using LLProcess, expecting that it will + * write to the file passed as its sys.argv[1]. Retrieve that output. + * + * Until January 2012, LLProcess provided distressingly few + * mechanisms for a child process to communicate back to its caller -- + * not even its return code. We've introduced a convention by which we + * create an empty temp file, pass the name of that file to our child + * as sys.argv[1] and expect the script to write its output to that + * file. This function implements the C++ (parent process) side of + * that convention. + */ + std::string run_read() + { + NamedTempFile out("out", ""); // placeholder + // pass name of this temporary file to the script + mParams["args"].append(out.getName()); + run(); + // assuming the script wrote to that file, read it + return readfile(out.getName(), STRINGIZE("from " << mDesc << " script")); + } + + LLSD mParams; + LLProcessPtr mPy; + std::string mDesc; + NamedTempFile mScript; +}; + +/// convenience function for PythonProcessLauncher::run() +template +static void python(const std::string& desc, const CONTENT& script) +{ + PythonProcessLauncher py(desc, script); + py.run(); +} + +/// convenience function for PythonProcessLauncher::run_read() +template +static std::string python_out(const std::string& desc, const CONTENT& script) +{ + PythonProcessLauncher py(desc, script); + return py.run_read(); +} + +/// Create a temporary directory and clean it up later. +class NamedTempDir: public boost::noncopyable +{ +public: + // Use python() function to create a temp directory: I've found + // nothing in either Boost.Filesystem or APR quite like Python's + // tempfile.mkdtemp(). + // Special extra bonus: on Mac, mkdtemp() reports a pathname + // starting with /var/folders/something, whereas that's really a + // symlink to /private/var/folders/something. Have to use + // realpath() to compare properly. + NamedTempDir(): + mPath(python_out("mkdtemp()", + "from __future__ import with_statement\n" + "import os.path, sys, tempfile\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write(os.path.realpath(tempfile.mkdtemp()))\n")) + {} + + ~NamedTempDir() + { + aprchk(apr_dir_remove(mPath.c_str(), gAPRPoolp)); + } + + std::string getName() const { return mPath; } + +private: + std::string mPath; +}; + +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + struct llprocess_data + { + LLAPRPool pool; + }; + typedef test_group llprocess_group; + typedef llprocess_group::object object; + llprocess_group llprocessgrp("llprocess"); + + struct Item + { + Item(): tries(0) {} + unsigned tries; + std::string which; + std::string what; + }; + +/*==========================================================================*| +#define tabent(symbol) { symbol, #symbol } + static struct ReasonCode + { + int code; + const char* name; + } reasons[] = + { + tabent(APR_OC_REASON_DEATH), + tabent(APR_OC_REASON_UNWRITABLE), + tabent(APR_OC_REASON_RESTART), + tabent(APR_OC_REASON_UNREGISTER), + tabent(APR_OC_REASON_LOST), + tabent(APR_OC_REASON_RUNNING) + }; +#undef tabent +|*==========================================================================*/ + + struct WaitInfo + { + WaitInfo(apr_proc_t* child_): + child(child_), + rv(-1), // we haven't yet called apr_proc_wait() + rc(0), + why(apr_exit_why_e(0)) + {} + apr_proc_t* child; // which subprocess + apr_status_t rv; // return from apr_proc_wait() + int rc; // child's exit code + apr_exit_why_e why; // APR_PROC_EXIT, APR_PROC_SIGNAL, APR_PROC_SIGNAL_CORE + }; + + void child_status_callback(int reason, void* data, int status) + { +/*==========================================================================*| + std::string reason_str; + BOOST_FOREACH(const ReasonCode& rcp, reasons) + { + if (reason == rcp.code) + { + reason_str = rcp.name; + break; + } + } + if (reason_str.empty()) + { + reason_str = STRINGIZE("unknown reason " << reason); + } + std::cout << "child_status_callback(" << reason_str << ")\n"; +|*==========================================================================*/ + + if (reason == APR_OC_REASON_DEATH || reason == APR_OC_REASON_LOST) + { + // Somewhat oddly, APR requires that you explicitly unregister + // even when it already knows the child has terminated. + apr_proc_other_child_unregister(data); + + WaitInfo* wi(static_cast(data)); + // It's just wrong to call apr_proc_wait() here. The only way APR + // knows to call us with APR_OC_REASON_DEATH is that it's already + // reaped this child process, so calling wait() will only produce + // "huh?" from the OS. We must rely on the status param passed in, + // which unfortunately comes straight from the OS wait() call. +// wi->rv = apr_proc_wait(wi->child, &wi->rc, &wi->why, APR_NOWAIT); + wi->rv = APR_CHILD_DONE; // fake apr_proc_wait() results +#if defined(LL_WINDOWS) + wi->why = APR_PROC_EXIT; + wi->rc = status; // no encoding on Windows (no signals) +#else // Posix + if (WIFEXITED(status)) + { + wi->why = APR_PROC_EXIT; + wi->rc = WEXITSTATUS(status); + } + else if (WIFSIGNALED(status)) + { + wi->why = APR_PROC_SIGNAL; + wi->rc = WTERMSIG(status); + } + else // uh, shouldn't happen? + { + wi->why = APR_PROC_EXIT; + wi->rc = status; // someone else will have to decode + } +#endif // Posix + } + } + + template<> template<> + void object::test<1>() + { + set_test_name("raw APR nonblocking I/O"); + + // Create a script file in a temporary place. + NamedTempFile script("py", + "import sys" EOL + "import time" EOL + EOL + "time.sleep(2)" EOL + "print >>sys.stdout, 'stdout after wait'" EOL + "sys.stdout.flush()" EOL + "time.sleep(2)" EOL + "print >>sys.stderr, 'stderr after wait'" EOL + "sys.stderr.flush()" EOL + ); + + // Arrange to track the history of our interaction with child: what we + // fetched, which pipe it came from, how many tries it took before we + // got it. + std::vector history; + history.push_back(Item()); + + // Run the child process. + apr_procattr_t *procattr = NULL; + aprchk(apr_procattr_create(&procattr, pool.getAPRPool())); + aprchk(apr_procattr_io_set(procattr, APR_CHILD_BLOCK, APR_CHILD_BLOCK, APR_CHILD_BLOCK)); + aprchk(apr_procattr_cmdtype_set(procattr, APR_PROGRAM_PATH)); + + std::vector argv; + apr_proc_t child; + argv.push_back("python"); + // Have to have a named copy of this std::string so its c_str() value + // will persist. + std::string scriptname(script.getName()); + argv.push_back(scriptname.c_str()); + argv.push_back(NULL); + + aprchk(apr_proc_create(&child, argv[0], + &argv[0], + NULL, // if we wanted to pass explicit environment + procattr, + pool.getAPRPool())); + + // We do not want this child process to outlive our APR pool. On + // destruction of the pool, forcibly kill the process. Tell APR to try + // SIGTERM and wait 3 seconds. If that didn't work, use SIGKILL. + apr_pool_note_subprocess(pool.getAPRPool(), &child, APR_KILL_AFTER_TIMEOUT); + + // arrange to call child_status_callback() + WaitInfo wi(&child); + apr_proc_other_child_register(&child, child_status_callback, &wi, child.in, pool.getAPRPool()); + + // TODO: + // Stuff child.in until it (would) block to verify EWOULDBLOCK/EAGAIN. + // Have child script clear it later, then write one more line to prove + // that it gets through. + + // Monitor two different output pipes. Because one will be closed + // before the other, keep them in a list so we can drop whichever of + // them is closed first. + typedef std::pair DescFile; + typedef std::list DescFileList; + DescFileList outfiles; + outfiles.push_back(DescFile("out", child.out)); + outfiles.push_back(DescFile("err", child.err)); + + while (! outfiles.empty()) + { + // This peculiar for loop is designed to let us erase(dfli). With + // a list, that invalidates only dfli itself -- but even so, we + // lose the ability to increment it for the next item. So at the + // top of every loop, while dfli is still valid, increment + // dflnext. Then before the next iteration, set dfli to dflnext. + for (DescFileList::iterator + dfli(outfiles.begin()), dflnext(outfiles.begin()), dflend(outfiles.end()); + dfli != dflend; dfli = dflnext) + { + // Only valid to increment dflnext once we're sure it's not + // already at dflend. + ++dflnext; + + char buf[4096]; + + apr_status_t rv = apr_file_gets(buf, sizeof(buf), dfli->second); + if (APR_STATUS_IS_EOF(rv)) + { +// std::cout << "(EOF on " << dfli->first << ")\n"; +// history.back().which = dfli->first; +// history.back().what = "*eof*"; +// history.push_back(Item()); + outfiles.erase(dfli); + continue; + } + if (rv == EWOULDBLOCK || rv == EAGAIN) + { +// std::cout << "(waiting; apr_file_gets(" << dfli->first << ") => " << rv << ": " << manager.strerror(rv) << ")\n"; + ++history.back().tries; + continue; + } + aprchk_("apr_file_gets(buf, sizeof(buf), dfli->second)", rv); + // Is it even possible to get APR_SUCCESS but read 0 bytes? + // Hope not, but defend against that anyway. + if (buf[0]) + { +// std::cout << dfli->first << ": " << buf; + history.back().which = dfli->first; + history.back().what.append(buf); + if (buf[strlen(buf) - 1] == '\n') + history.push_back(Item()); + else + { + // Just for pretty output... if we only read a partial + // line, terminate it. +// std::cout << "...\n"; + } + } + } + // Do this once per tick, as we expect the viewer will + apr_proc_other_child_refresh_all(APR_OC_REASON_RUNNING); + sleep(1); + } + apr_file_close(child.in); + apr_file_close(child.out); + apr_file_close(child.err); + + // Okay, we've broken the loop because our pipes are all closed. If we + // haven't yet called wait, give the callback one more chance. This + // models the fact that unlike this small test program, the viewer + // will still be running. + if (wi.rv == -1) + { + std::cout << "last gasp apr_proc_other_child_refresh_all()\n"; + apr_proc_other_child_refresh_all(APR_OC_REASON_RUNNING); + } + + if (wi.rv == -1) + { + std::cout << "child_status_callback(APR_OC_REASON_DEATH) wasn't called" << std::endl; + wi.rv = apr_proc_wait(wi.child, &wi.rc, &wi.why, APR_NOWAIT); + } +// std::cout << "child done: rv = " << rv << " (" << manager.strerror(rv) << "), why = " << why << ", rc = " << rc << '\n'; + aprchk_("apr_proc_wait(wi->child, &wi->rc, &wi->why, APR_NOWAIT)", wi.rv, APR_CHILD_DONE); + ensure_equals_(wi.why, APR_PROC_EXIT); + ensure_equals_(wi.rc, 0); + + // Beyond merely executing all the above successfully, verify that we + // obtained expected output -- and that we duly got control while + // waiting, proving the non-blocking nature of these pipes. + try + { + unsigned i = 0; + ensure("blocking I/O on child pipe (0)", history[i].tries); + ensure_equals_(history[i].which, "out"); + ensure_equals_(history[i].what, "stdout after wait" EOL); +// ++i; +// ensure_equals_(history[i].which, "out"); +// ensure_equals_(history[i].what, "*eof*"); + ++i; + ensure("blocking I/O on child pipe (1)", history[i].tries); + ensure_equals_(history[i].which, "err"); + ensure_equals_(history[i].what, "stderr after wait" EOL); +// ++i; +// ensure_equals_(history[i].which, "err"); +// ensure_equals_(history[i].what, "*eof*"); + } + catch (const failure&) + { + std::cout << "History:\n"; + BOOST_FOREACH(const Item& item, history) + { + std::string what(item.what); + if ((! what.empty()) && what[what.length() - 1] == '\n') + { + what.erase(what.length() - 1); + if ((! what.empty()) && what[what.length() - 1] == '\r') + { + what.erase(what.length() - 1); + what.append("\\r"); + } + what.append("\\n"); + } + std::cout << " " << item.which << ": '" << what << "' (" + << item.tries << " tries)\n"; + } + std::cout << std::flush; + // re-raise same error; just want to enrich the output + throw; + } + } + + template<> template<> + void object::test<2>() + { + set_test_name("setWorkingDirectory()"); + // We want to test setWorkingDirectory(). But what directory is + // guaranteed to exist on every machine, under every OS? Have to + // create one. Naturally, ensure we clean it up when done. + NamedTempDir tempdir; + PythonProcessLauncher py("getcwd()", + "from __future__ import with_statement\n" + "import os, sys\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write(os.getcwd())\n"); + // Before running, call setWorkingDirectory() + py.mParams["cwd"] = tempdir.getName(); + ensure_equals("os.getcwd()", py.run_read(), tempdir.getName()); + } + + template<> template<> + void object::test<3>() + { + set_test_name("arguments"); + PythonProcessLauncher py("args", + "from __future__ import with_statement\n" + "import sys\n" + // note nonstandard output-file arg! + "with open(sys.argv[3], 'w') as f:\n" + " for arg in sys.argv[1:]:\n" + " print >>f, arg\n"); + // We expect that PythonProcessLauncher has already appended + // its own NamedTempFile to mParams["args"] (sys.argv[0]). + py.mParams["args"].append("first arg"); // sys.argv[1] + py.mParams["args"].append("second arg"); // sys.argv[2] + // run_read() appends() one more argument, hence [3] + std::string output(py.run_read()); + boost::split_iterator + li(output, boost::first_finder("\n")), lend; + ensure("didn't get first arg", li != lend); + std::string arg(li->begin(), li->end()); + ensure_equals(arg, "first arg"); + ++li; + ensure("didn't get second arg", li != lend); + arg.assign(li->begin(), li->end()); + ensure_equals(arg, "second arg"); + ++li; + ensure("didn't get output filename?!", li != lend); + arg.assign(li->begin(), li->end()); + ensure("output filename empty?!", ! arg.empty()); + ++li; + ensure("too many args", li == lend); + } + + template<> template<> + void object::test<4>() + { + set_test_name("explicit kill()"); + PythonProcessLauncher py("kill()", + "from __future__ import with_statement\n" + "import sys, time\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('ok')\n" + "# now sleep; expect caller to kill\n" + "time.sleep(120)\n" + "# if caller hasn't managed to kill by now, bad\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('bad')\n"); + NamedTempFile out("out", "not started"); + py.mParams["args"].append(out.getName()); + py.mPy = LLProcess::create(py.mParams); + ensure("couldn't launch kill() script", py.mPy); + // Wait for the script to wake up and do its first write + int i = 0, timeout = 60; + for ( ; i < timeout; ++i) + { + sleep(1); + if (readfile(out.getName(), "from kill() script") == "ok") + break; + } + // If we broke this loop because of the counter, something's wrong + ensure("script never started", i < timeout); + // script has performed its first write and should now be sleeping. + py.mPy->kill(); + // wait for the script to terminate... one way or another. + while (py.mPy->isRunning()) + { + sleep(1); + } + // If kill() failed, the script would have woken up on its own and + // overwritten the file with 'bad'. But if kill() succeeded, it should + // not have had that chance. + ensure_equals("kill() script output", readfile(out.getName()), "ok"); + } + + template<> template<> + void object::test<5>() + { + set_test_name("implicit kill()"); + NamedTempFile out("out", "not started"); + LLProcess::id pid(0); + { + PythonProcessLauncher py("kill()", + "from __future__ import with_statement\n" + "import sys, time\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('ok')\n" + "# now sleep; expect caller to kill\n" + "time.sleep(120)\n" + "# if caller hasn't managed to kill by now, bad\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('bad')\n"); + py.mParams["args"].append(out.getName()); + py.mPy = LLProcess::create(py.mParams); + ensure("couldn't launch kill() script", py.mPy); + // Capture id for later + pid = py.mPy->getProcessID(); + // Wait for the script to wake up and do its first write + int i = 0, timeout = 60; + for ( ; i < timeout; ++i) + { + sleep(1); + if (readfile(out.getName(), "from kill() script") == "ok") + break; + } + // If we broke this loop because of the counter, something's wrong + ensure("script never started", i < timeout); + // Script has performed its first write and should now be sleeping. + // Destroy the LLProcess, which should kill the child. + } + // wait for the script to terminate... one way or another. + while (LLProcess::isRunning(pid)) + { + sleep(1); + } + // If kill() failed, the script would have woken up on its own and + // overwritten the file with 'bad'. But if kill() succeeded, it should + // not have had that chance. + ensure_equals("kill() script output", readfile(out.getName()), "ok"); + } + + template<> template<> + void object::test<6>() + { + set_test_name("autokill"); + NamedTempFile from("from", "not started"); + NamedTempFile to("to", ""); + LLProcess::id pid(0); + { + PythonProcessLauncher py("autokill", + "from __future__ import with_statement\n" + "import sys, time\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('ok')\n" + "# wait for 'go' from test program\n" + "for i in xrange(60):\n" + " time.sleep(1)\n" + " with open(sys.argv[2]) as f:\n" + " go = f.read()\n" + " if go == 'go':\n" + " break\n" + "else:\n" + " with open(sys.argv[1], 'w') as f:\n" + " f.write('never saw go')\n" + " sys.exit(1)\n" + "# okay, saw 'go', write 'ack'\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('ack')\n"); + py.mParams["args"].append(from.getName()); + py.mParams["args"].append(to.getName()); + py.mParams["autokill"] = false; + py.mPy = LLProcess::create(py.mParams); + ensure("couldn't launch kill() script", py.mPy); + // Capture id for later + pid = py.mPy->getProcessID(); + // Wait for the script to wake up and do its first write + int i = 0, timeout = 60; + for ( ; i < timeout; ++i) + { + sleep(1); + if (readfile(from.getName(), "from autokill script") == "ok") + break; + } + // If we broke this loop because of the counter, something's wrong + ensure("script never started", i < timeout); + // Now destroy the LLProcess, which should NOT kill the child! + } + // If the destructor killed the child anyway, give it time to die + sleep(2); + // How do we know it's not terminated? By making it respond to + // a specific stimulus in a specific way. + { + std::ofstream outf(to.getName().c_str()); + outf << "go"; + } // flush and close. + // now wait for the script to terminate... one way or another. + while (LLProcess::isRunning(pid)) + { + sleep(1); + } + // If the LLProcess destructor implicitly called kill(), the + // script could not have written 'ack' as we expect. + ensure_equals("autokill script output", readfile(from.getName()), "ack"); + } +} // namespace tut diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp deleted file mode 100644 index 057f83631e..0000000000 --- a/indra/llcommon/tests/llprocesslauncher_test.cpp +++ /dev/null @@ -1,718 +0,0 @@ -/** - * @file llprocesslauncher_test.cpp - * @author Nat Goodspeed - * @date 2011-12-19 - * @brief Test for llprocesslauncher. - * - * $LicenseInfo:firstyear=2011&license=viewerlgpl$ - * Copyright (c) 2011, Linden Research, Inc. - * $/LicenseInfo$ - */ - -// Precompiled header -#include "linden_common.h" -// associated header -#include "llprocesslauncher.h" -// STL headers -#include -#include -// std headers -#include -// external library headers -#include "llapr.h" -#include "apr_thread_proc.h" -#include -#include -#include -#include -//#include -//#include -// other Linden headers -#include "../test/lltut.h" -#include "../test/manageapr.h" -#include "../test/namedtempfile.h" -#include "stringize.h" - -#if defined(LL_WINDOWS) -#define sleep(secs) _sleep((secs) * 1000) -#define EOL "\r\n" -#else -#define EOL "\n" -#include -#endif - -//namespace lambda = boost::lambda; - -// static instance of this manages APR init/cleanup -static ManageAPR manager; - -/***************************************************************************** -* Helpers -*****************************************************************************/ - -#define ensure_equals_(left, right) \ - ensure_equals(STRINGIZE(#left << " != " << #right), (left), (right)) - -#define aprchk(expr) aprchk_(#expr, (expr)) -static void aprchk_(const char* call, apr_status_t rv, apr_status_t expected=APR_SUCCESS) -{ - tut::ensure_equals(STRINGIZE(call << " => " << rv << ": " << manager.strerror(rv)), - rv, expected); -} - -/** - * Read specified file using std::getline(). It is assumed to be an error if - * the file is empty: don't use this function if that's an acceptable case. - * Last line will not end with '\n'; this is to facilitate the usual case of - * string compares with a single line of output. - * @param pathname The file to read. - * @param desc Optional description of the file for error message; - * defaults to "in " - */ -static std::string readfile(const std::string& pathname, const std::string& desc="") -{ - std::string use_desc(desc); - if (use_desc.empty()) - { - use_desc = STRINGIZE("in " << pathname); - } - std::ifstream inf(pathname.c_str()); - std::string output; - tut::ensure(STRINGIZE("No output " << use_desc), std::getline(inf, output)); - std::string more; - while (std::getline(inf, more)) - { - output += '\n' + more; - } - return output; -} - -/** - * Construct an LLProcessLauncher to run a Python script. - */ -struct PythonProcessLauncher -{ - /** - * @param desc Arbitrary description for error messages - * @param script Python script, any form acceptable to NamedTempFile, - * typically either a std::string or an expression of the form - * (lambda::_1 << "script content with " << variable_data) - */ - template - PythonProcessLauncher(const std::string& desc, const CONTENT& script): - mDesc(desc), - mScript("py", script) - { - const char* PYTHON(getenv("PYTHON")); - tut::ensure("Set $PYTHON to the Python interpreter", PYTHON); - - mPy.setExecutable(PYTHON); - mPy.addArgument(mScript.getName()); - } - - /// Run Python script and wait for it to complete. - void run() - { - tut::ensure_equals(STRINGIZE("Couldn't launch " << mDesc << " script"), - mPy.launch(), 0); - // One of the irritating things about LLProcessLauncher is that - // there's no API to wait for the child to terminate -- but given - // its use in our graphics-intensive interactive viewer, it's - // understandable. - while (mPy.isRunning()) - { - sleep(1); - } - } - - /** - * Run a Python script using LLProcessLauncher, expecting that it will - * write to the file passed as its sys.argv[1]. Retrieve that output. - * - * Until January 2012, LLProcessLauncher provided distressingly few - * mechanisms for a child process to communicate back to its caller -- - * not even its return code. We've introduced a convention by which we - * create an empty temp file, pass the name of that file to our child - * as sys.argv[1] and expect the script to write its output to that - * file. This function implements the C++ (parent process) side of - * that convention. - */ - std::string run_read() - { - NamedTempFile out("out", ""); // placeholder - // pass name of this temporary file to the script - mPy.addArgument(out.getName()); - run(); - // assuming the script wrote to that file, read it - return readfile(out.getName(), STRINGIZE("from " << mDesc << " script")); - } - - LLProcessLauncher mPy; - std::string mDesc; - NamedTempFile mScript; -}; - -/// convenience function for PythonProcessLauncher::run() -template -static void python(const std::string& desc, const CONTENT& script) -{ - PythonProcessLauncher py(desc, script); - py.run(); -} - -/// convenience function for PythonProcessLauncher::run_read() -template -static std::string python_out(const std::string& desc, const CONTENT& script) -{ - PythonProcessLauncher py(desc, script); - return py.run_read(); -} - -/// Create a temporary directory and clean it up later. -class NamedTempDir: public boost::noncopyable -{ -public: - // Use python() function to create a temp directory: I've found - // nothing in either Boost.Filesystem or APR quite like Python's - // tempfile.mkdtemp(). - // Special extra bonus: on Mac, mkdtemp() reports a pathname - // starting with /var/folders/something, whereas that's really a - // symlink to /private/var/folders/something. Have to use - // realpath() to compare properly. - NamedTempDir(): - mPath(python_out("mkdtemp()", - "from __future__ import with_statement\n" - "import os.path, sys, tempfile\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write(os.path.realpath(tempfile.mkdtemp()))\n")) - {} - - ~NamedTempDir() - { - aprchk(apr_dir_remove(mPath.c_str(), gAPRPoolp)); - } - - std::string getName() const { return mPath; } - -private: - std::string mPath; -}; - -/***************************************************************************** -* TUT -*****************************************************************************/ -namespace tut -{ - struct llprocesslauncher_data - { - LLAPRPool pool; - }; - typedef test_group llprocesslauncher_group; - typedef llprocesslauncher_group::object object; - llprocesslauncher_group llprocesslaunchergrp("llprocesslauncher"); - - struct Item - { - Item(): tries(0) {} - unsigned tries; - std::string which; - std::string what; - }; - -/*==========================================================================*| -#define tabent(symbol) { symbol, #symbol } - static struct ReasonCode - { - int code; - const char* name; - } reasons[] = - { - tabent(APR_OC_REASON_DEATH), - tabent(APR_OC_REASON_UNWRITABLE), - tabent(APR_OC_REASON_RESTART), - tabent(APR_OC_REASON_UNREGISTER), - tabent(APR_OC_REASON_LOST), - tabent(APR_OC_REASON_RUNNING) - }; -#undef tabent -|*==========================================================================*/ - - struct WaitInfo - { - WaitInfo(apr_proc_t* child_): - child(child_), - rv(-1), // we haven't yet called apr_proc_wait() - rc(0), - why(apr_exit_why_e(0)) - {} - apr_proc_t* child; // which subprocess - apr_status_t rv; // return from apr_proc_wait() - int rc; // child's exit code - apr_exit_why_e why; // APR_PROC_EXIT, APR_PROC_SIGNAL, APR_PROC_SIGNAL_CORE - }; - - void child_status_callback(int reason, void* data, int status) - { -/*==========================================================================*| - std::string reason_str; - BOOST_FOREACH(const ReasonCode& rcp, reasons) - { - if (reason == rcp.code) - { - reason_str = rcp.name; - break; - } - } - if (reason_str.empty()) - { - reason_str = STRINGIZE("unknown reason " << reason); - } - std::cout << "child_status_callback(" << reason_str << ")\n"; -|*==========================================================================*/ - - if (reason == APR_OC_REASON_DEATH || reason == APR_OC_REASON_LOST) - { - // Somewhat oddly, APR requires that you explicitly unregister - // even when it already knows the child has terminated. - apr_proc_other_child_unregister(data); - - WaitInfo* wi(static_cast(data)); - // It's just wrong to call apr_proc_wait() here. The only way APR - // knows to call us with APR_OC_REASON_DEATH is that it's already - // reaped this child process, so calling wait() will only produce - // "huh?" from the OS. We must rely on the status param passed in, - // which unfortunately comes straight from the OS wait() call. -// wi->rv = apr_proc_wait(wi->child, &wi->rc, &wi->why, APR_NOWAIT); - wi->rv = APR_CHILD_DONE; // fake apr_proc_wait() results -#if defined(LL_WINDOWS) - wi->why = APR_PROC_EXIT; - wi->rc = status; // no encoding on Windows (no signals) -#else // Posix - if (WIFEXITED(status)) - { - wi->why = APR_PROC_EXIT; - wi->rc = WEXITSTATUS(status); - } - else if (WIFSIGNALED(status)) - { - wi->why = APR_PROC_SIGNAL; - wi->rc = WTERMSIG(status); - } - else // uh, shouldn't happen? - { - wi->why = APR_PROC_EXIT; - wi->rc = status; // someone else will have to decode - } -#endif // Posix - } - } - - template<> template<> - void object::test<1>() - { - set_test_name("raw APR nonblocking I/O"); - - // Create a script file in a temporary place. - NamedTempFile script("py", - "import sys" EOL - "import time" EOL - EOL - "time.sleep(2)" EOL - "print >>sys.stdout, 'stdout after wait'" EOL - "sys.stdout.flush()" EOL - "time.sleep(2)" EOL - "print >>sys.stderr, 'stderr after wait'" EOL - "sys.stderr.flush()" EOL - ); - - // Arrange to track the history of our interaction with child: what we - // fetched, which pipe it came from, how many tries it took before we - // got it. - std::vector history; - history.push_back(Item()); - - // Run the child process. - apr_procattr_t *procattr = NULL; - aprchk(apr_procattr_create(&procattr, pool.getAPRPool())); - aprchk(apr_procattr_io_set(procattr, APR_CHILD_BLOCK, APR_CHILD_BLOCK, APR_CHILD_BLOCK)); - aprchk(apr_procattr_cmdtype_set(procattr, APR_PROGRAM_PATH)); - - std::vector argv; - apr_proc_t child; - argv.push_back("python"); - // Have to have a named copy of this std::string so its c_str() value - // will persist. - std::string scriptname(script.getName()); - argv.push_back(scriptname.c_str()); - argv.push_back(NULL); - - aprchk(apr_proc_create(&child, argv[0], - &argv[0], - NULL, // if we wanted to pass explicit environment - procattr, - pool.getAPRPool())); - - // We do not want this child process to outlive our APR pool. On - // destruction of the pool, forcibly kill the process. Tell APR to try - // SIGTERM and wait 3 seconds. If that didn't work, use SIGKILL. - apr_pool_note_subprocess(pool.getAPRPool(), &child, APR_KILL_AFTER_TIMEOUT); - - // arrange to call child_status_callback() - WaitInfo wi(&child); - apr_proc_other_child_register(&child, child_status_callback, &wi, child.in, pool.getAPRPool()); - - // TODO: - // Stuff child.in until it (would) block to verify EWOULDBLOCK/EAGAIN. - // Have child script clear it later, then write one more line to prove - // that it gets through. - - // Monitor two different output pipes. Because one will be closed - // before the other, keep them in a list so we can drop whichever of - // them is closed first. - typedef std::pair DescFile; - typedef std::list DescFileList; - DescFileList outfiles; - outfiles.push_back(DescFile("out", child.out)); - outfiles.push_back(DescFile("err", child.err)); - - while (! outfiles.empty()) - { - // This peculiar for loop is designed to let us erase(dfli). With - // a list, that invalidates only dfli itself -- but even so, we - // lose the ability to increment it for the next item. So at the - // top of every loop, while dfli is still valid, increment - // dflnext. Then before the next iteration, set dfli to dflnext. - for (DescFileList::iterator - dfli(outfiles.begin()), dflnext(outfiles.begin()), dflend(outfiles.end()); - dfli != dflend; dfli = dflnext) - { - // Only valid to increment dflnext once we're sure it's not - // already at dflend. - ++dflnext; - - char buf[4096]; - - apr_status_t rv = apr_file_gets(buf, sizeof(buf), dfli->second); - if (APR_STATUS_IS_EOF(rv)) - { -// std::cout << "(EOF on " << dfli->first << ")\n"; -// history.back().which = dfli->first; -// history.back().what = "*eof*"; -// history.push_back(Item()); - outfiles.erase(dfli); - continue; - } - if (rv == EWOULDBLOCK || rv == EAGAIN) - { -// std::cout << "(waiting; apr_file_gets(" << dfli->first << ") => " << rv << ": " << manager.strerror(rv) << ")\n"; - ++history.back().tries; - continue; - } - aprchk_("apr_file_gets(buf, sizeof(buf), dfli->second)", rv); - // Is it even possible to get APR_SUCCESS but read 0 bytes? - // Hope not, but defend against that anyway. - if (buf[0]) - { -// std::cout << dfli->first << ": " << buf; - history.back().which = dfli->first; - history.back().what.append(buf); - if (buf[strlen(buf) - 1] == '\n') - history.push_back(Item()); - else - { - // Just for pretty output... if we only read a partial - // line, terminate it. -// std::cout << "...\n"; - } - } - } - // Do this once per tick, as we expect the viewer will - apr_proc_other_child_refresh_all(APR_OC_REASON_RUNNING); - sleep(1); - } - apr_file_close(child.in); - apr_file_close(child.out); - apr_file_close(child.err); - - // Okay, we've broken the loop because our pipes are all closed. If we - // haven't yet called wait, give the callback one more chance. This - // models the fact that unlike this small test program, the viewer - // will still be running. - if (wi.rv == -1) - { - std::cout << "last gasp apr_proc_other_child_refresh_all()\n"; - apr_proc_other_child_refresh_all(APR_OC_REASON_RUNNING); - } - - if (wi.rv == -1) - { - std::cout << "child_status_callback(APR_OC_REASON_DEATH) wasn't called" << std::endl; - wi.rv = apr_proc_wait(wi.child, &wi.rc, &wi.why, APR_NOWAIT); - } -// std::cout << "child done: rv = " << rv << " (" << manager.strerror(rv) << "), why = " << why << ", rc = " << rc << '\n'; - aprchk_("apr_proc_wait(wi->child, &wi->rc, &wi->why, APR_NOWAIT)", wi.rv, APR_CHILD_DONE); - ensure_equals_(wi.why, APR_PROC_EXIT); - ensure_equals_(wi.rc, 0); - - // Beyond merely executing all the above successfully, verify that we - // obtained expected output -- and that we duly got control while - // waiting, proving the non-blocking nature of these pipes. - try - { - unsigned i = 0; - ensure("blocking I/O on child pipe (0)", history[i].tries); - ensure_equals_(history[i].which, "out"); - ensure_equals_(history[i].what, "stdout after wait" EOL); -// ++i; -// ensure_equals_(history[i].which, "out"); -// ensure_equals_(history[i].what, "*eof*"); - ++i; - ensure("blocking I/O on child pipe (1)", history[i].tries); - ensure_equals_(history[i].which, "err"); - ensure_equals_(history[i].what, "stderr after wait" EOL); -// ++i; -// ensure_equals_(history[i].which, "err"); -// ensure_equals_(history[i].what, "*eof*"); - } - catch (const failure&) - { - std::cout << "History:\n"; - BOOST_FOREACH(const Item& item, history) - { - std::string what(item.what); - if ((! what.empty()) && what[what.length() - 1] == '\n') - { - what.erase(what.length() - 1); - if ((! what.empty()) && what[what.length() - 1] == '\r') - { - what.erase(what.length() - 1); - what.append("\\r"); - } - what.append("\\n"); - } - std::cout << " " << item.which << ": '" << what << "' (" - << item.tries << " tries)\n"; - } - std::cout << std::flush; - // re-raise same error; just want to enrich the output - throw; - } - } - - template<> template<> - void object::test<2>() - { - set_test_name("set/getExecutable()"); - LLProcessLauncher child; - child.setExecutable("nonsense string"); - ensure_equals("setExecutable() 0", child.getExecutable(), "nonsense string"); - child.setExecutable("python"); - ensure_equals("setExecutable() 1", child.getExecutable(), "python"); - } - - template<> template<> - void object::test<3>() - { - set_test_name("setWorkingDirectory()"); - // We want to test setWorkingDirectory(). But what directory is - // guaranteed to exist on every machine, under every OS? Have to - // create one. Naturally, ensure we clean it up when done. - NamedTempDir tempdir; - PythonProcessLauncher py("getcwd()", - "from __future__ import with_statement\n" - "import os, sys\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write(os.getcwd())\n"); - // Before running, call setWorkingDirectory() - py.mPy.setWorkingDirectory(tempdir.getName()); - ensure_equals("os.getcwd()", py.run_read(), tempdir.getName()); - } - - template<> template<> - void object::test<4>() - { - set_test_name("clearArguments()"); - PythonProcessLauncher py("args", - "from __future__ import with_statement\n" - "import sys\n" - // note nonstandard output-file arg! - "with open(sys.argv[3], 'w') as f:\n" - " for arg in sys.argv[1:]:\n" - " print >>f, arg\n"); - // We expect that PythonProcessLauncher has already called - // addArgument() with the name of its own NamedTempFile. But let's - // change it up. - py.mPy.clearArguments(); - // re-add script pathname - py.mPy.addArgument(py.mScript.getName()); // sys.argv[0] - py.mPy.addArgument("first arg"); // sys.argv[1] - py.mPy.addArgument("second arg"); // sys.argv[2] - // run_read() calls addArgument() one more time, hence [3] - std::string output(py.run_read()); - boost::split_iterator - li(output, boost::first_finder("\n")), lend; - ensure("didn't get first arg", li != lend); - std::string arg(li->begin(), li->end()); - ensure_equals(arg, "first arg"); - ++li; - ensure("didn't get second arg", li != lend); - arg.assign(li->begin(), li->end()); - ensure_equals(arg, "second arg"); - ++li; - ensure("didn't get output filename?!", li != lend); - arg.assign(li->begin(), li->end()); - ensure("output filename empty?!", ! arg.empty()); - ++li; - ensure("too many args", li == lend); - } - - template<> template<> - void object::test<5>() - { - set_test_name("explicit kill()"); - PythonProcessLauncher py("kill()", - "from __future__ import with_statement\n" - "import sys, time\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write('ok')\n" - "# now sleep; expect caller to kill\n" - "time.sleep(120)\n" - "# if caller hasn't managed to kill by now, bad\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write('bad')\n"); - NamedTempFile out("out", "not started"); - py.mPy.addArgument(out.getName()); - ensure_equals("couldn't launch kill() script", py.mPy.launch(), 0); - // Wait for the script to wake up and do its first write - int i = 0, timeout = 60; - for ( ; i < timeout; ++i) - { - sleep(1); - if (readfile(out.getName(), "from kill() script") == "ok") - break; - } - // If we broke this loop because of the counter, something's wrong - ensure("script never started", i < timeout); - // script has performed its first write and should now be sleeping. - py.mPy.kill(); - // wait for the script to terminate... one way or another. - while (py.mPy.isRunning()) - { - sleep(1); - } - // If kill() failed, the script would have woken up on its own and - // overwritten the file with 'bad'. But if kill() succeeded, it should - // not have had that chance. - ensure_equals("kill() script output", readfile(out.getName()), "ok"); - } - - template<> template<> - void object::test<6>() - { - set_test_name("implicit kill()"); - NamedTempFile out("out", "not started"); - LLProcessLauncher::ll_pid_t pid(0); - { - PythonProcessLauncher py("kill()", - "from __future__ import with_statement\n" - "import sys, time\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write('ok')\n" - "# now sleep; expect caller to kill\n" - "time.sleep(120)\n" - "# if caller hasn't managed to kill by now, bad\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write('bad')\n"); - py.mPy.addArgument(out.getName()); - ensure_equals("couldn't launch kill() script", py.mPy.launch(), 0); - // Capture ll_pid_t for later - pid = py.mPy.getProcessID(); - // Wait for the script to wake up and do its first write - int i = 0, timeout = 60; - for ( ; i < timeout; ++i) - { - sleep(1); - if (readfile(out.getName(), "from kill() script") == "ok") - break; - } - // If we broke this loop because of the counter, something's wrong - ensure("script never started", i < timeout); - // Script has performed its first write and should now be sleeping. - // Destroy the LLProcessLauncher, which should kill the child. - } - // wait for the script to terminate... one way or another. - while (LLProcessLauncher::isRunning(pid)) - { - sleep(1); - } - // If kill() failed, the script would have woken up on its own and - // overwritten the file with 'bad'. But if kill() succeeded, it should - // not have had that chance. - ensure_equals("kill() script output", readfile(out.getName()), "ok"); - } - - template<> template<> - void object::test<7>() - { - set_test_name("orphan()"); - NamedTempFile from("from", "not started"); - NamedTempFile to("to", ""); - LLProcessLauncher::ll_pid_t pid(0); - { - PythonProcessLauncher py("orphan()", - "from __future__ import with_statement\n" - "import sys, time\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write('ok')\n" - "# wait for 'go' from test program\n" - "for i in xrange(60):\n" - " time.sleep(1)\n" - " with open(sys.argv[2]) as f:\n" - " go = f.read()\n" - " if go == 'go':\n" - " break\n" - "else:\n" - " with open(sys.argv[1], 'w') as f:\n" - " f.write('never saw go')\n" - " sys.exit(1)\n" - "# okay, saw 'go', write 'ack'\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write('ack')\n"); - py.mPy.addArgument(from.getName()); - py.mPy.addArgument(to.getName()); - ensure_equals("couldn't launch kill() script", py.mPy.launch(), 0); - // Capture ll_pid_t for later - pid = py.mPy.getProcessID(); - // Wait for the script to wake up and do its first write - int i = 0, timeout = 60; - for ( ; i < timeout; ++i) - { - sleep(1); - if (readfile(from.getName(), "from orphan() script") == "ok") - break; - } - // If we broke this loop because of the counter, something's wrong - ensure("script never started", i < timeout); - // Script has performed its first write and should now be waiting - // for us. Orphan it. - py.mPy.orphan(); - // Now destroy the LLProcessLauncher, which should NOT kill the child! - } - // If the destructor killed the child anyway, give it time to die - sleep(2); - // How do we know it's not terminated? By making it respond to - // a specific stimulus in a specific way. - { - std::ofstream outf(to.getName().c_str()); - outf << "go"; - } // flush and close. - // now wait for the script to terminate... one way or another. - while (LLProcessLauncher::isRunning(pid)) - { - sleep(1); - } - // If the LLProcessLauncher destructor implicitly called kill(), the - // script could not have written 'ack' as we expect. - ensure_equals("orphan() script output", readfile(from.getName()), "ack"); - } -} // namespace tut diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index 4359e9afb9..7756ba6226 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -40,7 +40,7 @@ typedef U32 uint32_t; #include #include #include -#include "llprocesslauncher.h" +#include "llprocess.h" #endif #include "boost/range.hpp" @@ -1557,14 +1557,15 @@ namespace tut } #else // LL_DARWIN, LL_LINUX - LLProcessLauncher py; - py.setExecutable(PYTHON); - py.addArgument(scriptfile.getName()); - ensure_equals(STRINGIZE("Couldn't launch " << desc << " script"), py.launch(), 0); + LLSD params; + params["executable"] = PYTHON; + params["args"].append(scriptfile.getName()); + LLProcessPtr py(LLProcess::create(params)); + ensure(STRINGIZE("Couldn't launch " << desc << " script"), py); // Implementing timeout would mean messing with alarm() and // catching SIGALRM... later maybe... int status(0); - if (waitpid(py.getProcessID(), &status, 0) == -1) + if (waitpid(py->getProcessID(), &status, 0) == -1) { int waitpid_errno(errno); ensure_equals(STRINGIZE("Couldn't retrieve rc from " << desc << " script: " -- cgit v1.2.3 From 47d94757075e338c480ba4d7d24948242a85a9bb Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 21 Jan 2012 11:45:15 -0500 Subject: Convert LLProcess consumers from LLSD to LLProcess::Params block. Using a Params block gives compile-time checking against attribute typos. One might inadvertently set myLLSD["autofill"] = false and only discover it when things behave strangely at runtime; but trying to set myParams.autofill will produce a compile error. However, it's excellent that the same LLProcess::create() method can accept either LLProcess::Params or a properly-constructed LLSD block. --- indra/llcommon/tests/llprocess_test.cpp | 26 +++++++++++++------------- indra/llcommon/tests/llsdserialize_test.cpp | 6 +++--- 2 files changed, 16 insertions(+), 16 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 55e22abd81..405540e436 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -107,8 +107,8 @@ struct PythonProcessLauncher const char* PYTHON(getenv("PYTHON")); tut::ensure("Set $PYTHON to the Python interpreter", PYTHON); - mParams["executable"] = PYTHON; - mParams["args"].append(mScript.getName()); + mParams.executable = PYTHON; + mParams.args.add(mScript.getName()); } /// Run Python script and wait for it to complete. @@ -142,13 +142,13 @@ struct PythonProcessLauncher { NamedTempFile out("out", ""); // placeholder // pass name of this temporary file to the script - mParams["args"].append(out.getName()); + mParams.args.add(out.getName()); run(); // assuming the script wrote to that file, read it return readfile(out.getName(), STRINGIZE("from " << mDesc << " script")); } - LLSD mParams; + LLProcess::Params mParams; LLProcessPtr mPy; std::string mDesc; NamedTempFile mScript; @@ -515,7 +515,7 @@ namespace tut "with open(sys.argv[1], 'w') as f:\n" " f.write(os.getcwd())\n"); // Before running, call setWorkingDirectory() - py.mParams["cwd"] = tempdir.getName(); + py.mParams.cwd = tempdir.getName(); ensure_equals("os.getcwd()", py.run_read(), tempdir.getName()); } @@ -531,9 +531,9 @@ namespace tut " for arg in sys.argv[1:]:\n" " print >>f, arg\n"); // We expect that PythonProcessLauncher has already appended - // its own NamedTempFile to mParams["args"] (sys.argv[0]). - py.mParams["args"].append("first arg"); // sys.argv[1] - py.mParams["args"].append("second arg"); // sys.argv[2] + // its own NamedTempFile to mParams.args (sys.argv[0]). + py.mParams.args.add("first arg"); // sys.argv[1] + py.mParams.args.add("second arg"); // sys.argv[2] // run_read() appends() one more argument, hence [3] std::string output(py.run_read()); boost::split_iterator @@ -568,7 +568,7 @@ namespace tut "with open(sys.argv[1], 'w') as f:\n" " f.write('bad')\n"); NamedTempFile out("out", "not started"); - py.mParams["args"].append(out.getName()); + py.mParams.args.add(out.getName()); py.mPy = LLProcess::create(py.mParams); ensure("couldn't launch kill() script", py.mPy); // Wait for the script to wake up and do its first write @@ -611,7 +611,7 @@ namespace tut "# if caller hasn't managed to kill by now, bad\n" "with open(sys.argv[1], 'w') as f:\n" " f.write('bad')\n"); - py.mParams["args"].append(out.getName()); + py.mParams.args.add(out.getName()); py.mPy = LLProcess::create(py.mParams); ensure("couldn't launch kill() script", py.mPy); // Capture id for later @@ -667,9 +667,9 @@ namespace tut "# okay, saw 'go', write 'ack'\n" "with open(sys.argv[1], 'w') as f:\n" " f.write('ack')\n"); - py.mParams["args"].append(from.getName()); - py.mParams["args"].append(to.getName()); - py.mParams["autokill"] = false; + py.mParams.args.add(from.getName()); + py.mParams.args.add(to.getName()); + py.mParams.autokill = false; py.mPy = LLProcess::create(py.mParams); ensure("couldn't launch kill() script", py.mPy); // Capture id for later diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index 7756ba6226..e625545763 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -1557,9 +1557,9 @@ namespace tut } #else // LL_DARWIN, LL_LINUX - LLSD params; - params["executable"] = PYTHON; - params["args"].append(scriptfile.getName()); + LLProcess::Params params; + params.executable = PYTHON; + params.args.add(scriptfile.getName()); LLProcessPtr py(LLProcess::create(params)); ensure(STRINGIZE("Couldn't launch " << desc << " script"), py); // Implementing timeout would mean messing with alarm() and -- cgit v1.2.3 From 85581eefa63d8f8e8c5132c4cd7e137f6cb88869 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 30 Jan 2012 12:11:44 -0500 Subject: Expose 'handle' as well as 'id' on LLProcess objects. On Posix, these and the corresponding getProcessID()/getProcessHandle() accessors produce the same pid_t value; but on Windows, it's useful to distinguish an int-like 'id' useful to human log readers versus an opaque 'handle' for passing to platform-specific API functions. So make the distinction in a platform-independent way. --- indra/llcommon/tests/llprocess_test.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 405540e436..4ad45bdf27 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -599,7 +599,7 @@ namespace tut { set_test_name("implicit kill()"); NamedTempFile out("out", "not started"); - LLProcess::id pid(0); + LLProcess::handle phandle(0); { PythonProcessLauncher py("kill()", "from __future__ import with_statement\n" @@ -614,8 +614,8 @@ namespace tut py.mParams.args.add(out.getName()); py.mPy = LLProcess::create(py.mParams); ensure("couldn't launch kill() script", py.mPy); - // Capture id for later - pid = py.mPy->getProcessID(); + // Capture handle for later + phandle = py.mPy->getProcessHandle(); // Wait for the script to wake up and do its first write int i = 0, timeout = 60; for ( ; i < timeout; ++i) @@ -630,7 +630,7 @@ namespace tut // Destroy the LLProcess, which should kill the child. } // wait for the script to terminate... one way or another. - while (LLProcess::isRunning(pid)) + while (LLProcess::isRunning(phandle)) { sleep(1); } @@ -646,7 +646,7 @@ namespace tut set_test_name("autokill"); NamedTempFile from("from", "not started"); NamedTempFile to("to", ""); - LLProcess::id pid(0); + LLProcess::handle phandle(0); { PythonProcessLauncher py("autokill", "from __future__ import with_statement\n" @@ -672,8 +672,8 @@ namespace tut py.mParams.autokill = false; py.mPy = LLProcess::create(py.mParams); ensure("couldn't launch kill() script", py.mPy); - // Capture id for later - pid = py.mPy->getProcessID(); + // Capture handle for later + phandle = py.mPy->getProcessHandle(); // Wait for the script to wake up and do its first write int i = 0, timeout = 60; for ( ; i < timeout; ++i) @@ -695,7 +695,7 @@ namespace tut outf << "go"; } // flush and close. // now wait for the script to terminate... one way or another. - while (LLProcess::isRunning(pid)) + while (LLProcess::isRunning(phandle)) { sleep(1); } -- cgit v1.2.3 From aafb03b29f5166e8978931ad8b717be32d942836 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 7 Feb 2012 10:53:23 -0500 Subject: Convert LLProcess implementation from platform-specific to using APR. Include logic to engage Linden apr_procattr_autokill_set() extension: on Windows, magic CreateProcess() flag must be pushed down into apr_proc_create() level. When using an APR package without that extension, present implementation should lock (e.g.) SLVoice.exe lifespan to viewer's on Windows XP but probably won't on Windows 7: need magic flag on CreateProcess(). Using APR child-termination callback requires us to define state (e.g. LLProcess::RUNNING). Take the opportunity to present Status, capturing state and (if terminated) rc or signal number; but since most of the time all caller really wants is to log the outcome, also present status string, encapsulating logic to examine state and describe exited-with-rc vs. killed-by-signal. New Status logic may report clearer results in the case of a Windows child process killed by exception. Clarify that static LLProcess::isRunning(handle) overload is only for use when the original LLProcess object has been destroyed: really only for unit tests. We necessarily retain our original platform-specific implementations for just that one method. (Nonstatic isRunning() no longer calls static method.) Clarify log output from llprocess_test.cpp in a couple places. --- indra/llcommon/tests/llprocess_test.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 4ad45bdf27..60ed12ad6a 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -630,7 +630,7 @@ namespace tut // Destroy the LLProcess, which should kill the child. } // wait for the script to terminate... one way or another. - while (LLProcess::isRunning(phandle)) + while (LLProcess::isRunning(phandle, "kill() script")) { sleep(1); } @@ -643,7 +643,7 @@ namespace tut template<> template<> void object::test<6>() { - set_test_name("autokill"); + set_test_name("autokill=false"); NamedTempFile from("from", "not started"); NamedTempFile to("to", ""); LLProcess::handle phandle(0); @@ -695,7 +695,7 @@ namespace tut outf << "go"; } // flush and close. // now wait for the script to terminate... one way or another. - while (LLProcess::isRunning(phandle)) + while (LLProcess::isRunning(phandle, "autokill script")) { sleep(1); } -- cgit v1.2.3 From 32e11494ffde368b2ac166e16dc294d66a18492f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 7 Feb 2012 12:28:57 -0500 Subject: Use os.path.normcase(os.path.normpath()) when comparing directories. Once again we've been bitten by comparison failure between "c:\somepath" and "C:\somepath". Normalize paths in both Python helper scripts to make that comparison more robust. --- indra/llcommon/tests/llprocess_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 60ed12ad6a..6d2292fb88 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -186,7 +186,7 @@ public: "from __future__ import with_statement\n" "import os.path, sys, tempfile\n" "with open(sys.argv[1], 'w') as f:\n" - " f.write(os.path.realpath(tempfile.mkdtemp()))\n")) + " f.write(os.path.normcase(os.path.normpath(os.path.realpath(tempfile.mkdtemp()))))\n")) {} ~NamedTempDir() @@ -513,7 +513,7 @@ namespace tut "from __future__ import with_statement\n" "import os, sys\n" "with open(sys.argv[1], 'w') as f:\n" - " f.write(os.getcwd())\n"); + " f.write(os.path.normcase(os.path.normpath(os.getcwd())))\n"); // Before running, call setWorkingDirectory() py.mParams.cwd = tempdir.getName(); ensure_equals("os.getcwd()", py.run_read(), tempdir.getName()); -- cgit v1.2.3