From cf7c6f93f28534fee2c13e29501b6ab6e7b77d61 Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Fri, 23 Dec 2011 15:23:03 -0500
Subject: [PATCH] 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.
---
.../llcommon/tests/llprocesslauncher_test.cpp | 43 ++++++++++++-------
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp
index bd7666313eb..d6d05ed7693 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 <vector>
+#include <list>
// std headers
#include <errno.h>
// 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<std::string, apr_file_t*> DescFile;
- typedef std::vector<DescFile> DescFileVec;
- DescFileVec outfiles;
+ typedef std::list<DescFile> 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';
--
GitLab