Skip to content
Snippets Groups Projects
  1. Mar 29, 2013
  2. Jun 06, 2012
    • Nat Goodspeed's avatar
      MAINT-1144: Defend against NULL LLPluginProcessParent::mProcess. · 166f75d9
      Nat Goodspeed authored
      The change from LLProcessLauncher to LLProcess introduces the possibility of a
      NULL (default-constructed) LLProcessPtr. Add certain static LLProcess methods
      accepting LLProcessPtr, forwarding to nonstatic method when non-NULL but doing
      something reasonable with NULL. Use these methods in LLPLuginProcessParent.
      166f75d9
  3. May 09, 2012
    • Nat Goodspeed's avatar
      CHOP-900: Use new apr_procattr_constrain_handle_set() extension. · d29f920c
      Nat Goodspeed authored
      Now LLProcess explicitly requests APR to limit the handles passed to any child
      process, instead of wantonly passing whatever happens to be lying around the
      parent process at the time.
      This requires the latest APR build.
      Also revert LLUpdateDownloader::Implementation::mDownloadStream to llofstream
      (as in rev 1878a57aebd7) instead of apr_file_t*. Using APR for that file was a
      Band-Aid -- a single whacked mole -- for the problem more systemically
      addressed by apr_procattr_constrain_handle_set().
      d29f920c
  4. Apr 23, 2012
    • Nat Goodspeed's avatar
      IQA-463: Use APR file I/O for downloaded viewer installer .exe. · 78816bb1
      Nat Goodspeed authored
      On Windows, calling CreateProcess(bInheritHandles=FALSE) is the wrong idea. In
      that case, CreateProcess() passes NO handles -- even the files you've
      explicitly designated as the child's stdin, stdout, stderr in the STARTUPINFO
      struct! Remove LLProcess code to tweak bInheritHandles; we should also remove
      the corresponding (useless) APR extension.
      Instead, given that the Windows file-locking problem we've observed is
      specific to the viewer installer .exe file downloaded by the background
      updater logic, use APR file I/O for that specific file. Empirically, both
      llofstream and std::ofstream seem to make the open file handle inheritable;
      but apr_file_open() documentation says: "By default, the returned file
      descriptor will not be inherited by child processes created by
      apr_proc_create()." And indeed, it does appear to sidestep the locking problem.
      78816bb1
  5. Apr 18, 2012
    • Nat Goodspeed's avatar
      IQA-463: Make LLProcess call apr_procattr_inherit_set() extension. · 38e23bb0
      Nat Goodspeed authored
      On Windows, Bad Things happen when apr_proc_create() is allowed to pass TRUE
      to CreateProcess(bInheritHandles). For instance, the open handle for a new
      installer executable file being downloaded by the background updater gets
      inadvertently passed to a couple slplugin.exe instances. When the viewer
      finishes downloading, closes the file and tries to remove it, Windows balks
      because the file is still open by another process. Require an apr_suite
      package that includes the new Linden apr_procattr_inherit_set() extension, and
      call it to turn off CreateProcess(bInheritHandles).
      38e23bb0
  6. Mar 15, 2012
  7. Mar 13, 2012
    • Nat Goodspeed's avatar
      Protect LLProcess destructor when run after APR shutdown. · ab7fb594
      Nat Goodspeed authored
      A static LLProcessPtr variable won't be destroyed until after procedural code
      has shut down APR. The trouble is that LLProcess's destructor unregisters
      itself from APR -- and, for an autokill LLProcess, attempts to kill the child
      process. All that is ill-advised after APR shutdown.
      Disable use of apr_pool_note_subprocess() mechanism. This should be another
      viable way of coping with static autokill LLProcessPtr variables: when the
      designated APR pool is cleaned up, APR promises to kill the child process. But
      whether it's an APR bug or a calling error, the present (now disabled) call in
      LLProcess results in OUR process, the viewer, getting SIGTERM when it asks to
      clean up the global APR pool.
      ab7fb594
  8. Mar 05, 2012
  9. Mar 03, 2012
    • Nat Goodspeed's avatar
      Break large buffer into chunks to write to LLProcess child pipe. · 75f41254
      Nat Goodspeed authored
      On Windows we ran into trouble trying to write a biggish (~1 MB) buffer of
      data to the child process's stdin pipe with a single apr_file_write() call.
      The child actually received corrupted data -- suggesting a possible bug in
      either APR or Windows pipes; the same test driving the same logic worked fine
      on Mac and Linux. Empirically, iterating over chunks of the buffered data is
      more robust.
      75f41254
  10. Mar 02, 2012
    • Nat Goodspeed's avatar
      Clarify LLProcess debug log message about reading from child pipe. · 260883c1
      Nat Goodspeed authored
      Previous "read N of M bytes" wording implied that the child had M bytes to
      send, but we only read N of them. In reality we have no idea how many bytes
      the child is trying to send, only how many the OS is willing to deliver at
      this moment. To me, "filled N of M bytes" more clearly implies that M is the
      buffer size.
      260883c1
  11. Mar 01, 2012
  12. Feb 29, 2012
    • Nat Goodspeed's avatar
      When constructing a pipe to child stdin on Posix, ignore SIGPIPE. · 40dc3e0d
      Nat Goodspeed authored
      We can't count on every child process reading everything we try to write to
      it. And if the child terminates with WritePipe data still pending, unless we
      explicitly suppress it, Posix will hit us with SIGPIPE. That would terminate
      the calling process, boom. "Ignoring" it means APR gets the correct errno,
      passes it back to us, we log it, etc.
      40dc3e0d
    • Nat Goodspeed's avatar
      Guarantee LLProcess::Params::postend listener any ReadPipe data. · 3649eda6
      Nat Goodspeed authored
      Previously one might get process-terminated notification but still have to
      wait for the child process's final data to arrive on one or more ReadPipes.
      That required complex consumer timing logic to handle incomplete pending
      ReadPipe data, e.g. a partial last line with no terminating newline. New code
      guarantees that by the time LLProcess sends process-terminated notification,
      all pending pipe data will have been buffered in ReadPipes.
      Document LLProcess::ReadPipe::getPump() notification event; add "eof" key.
      Add LLProcess::ReadPipe::getline() and read() convenience methods.
      Add static LLProcess::getline() and basename() convenience methods, publishing
      logic already present elsewhere.
      Use ReadPipe::getline() and read() in unit tests.
      Add unit test for "eof" event on ReadPipe::getPump().
      Add unit test verifying that final data have been buffered by termination
      notification event.
      3649eda6
  13. Feb 23, 2012
    • Nat Goodspeed's avatar
      Tighten up LLProcess pipe support, per Richard's code review. · 14ddc647
      Nat Goodspeed authored
      Clarify wording in some of the doc comments; be a bit more explicit about some
      of the parameter fields.
      Make some query methods 'const'.
      Change default LLProcess::ReadPipe::getLimit() value to 0: don't post any
      incoming data with notification event unless caller requests it. But do post
      pertinent FILESLOT in case caller reuses same listener for both stdout and
      stderr.
      Use more idiomatic, readable syntax for accessing LLProcess::Params data.
      14ddc647
  14. Feb 20, 2012
    • Nat Goodspeed's avatar
      Let LLProcess consumer specify desired description for logging. · 999484a6
      Nat Goodspeed authored
      If caller runs (e.g.) a Python script, it's not very helpful to a human log
      reader to keep seeing LLProcess instances logged as /pathname/to/python (pid).
      If caller is aware, the code can at least use the script name as the desc --
      or maybe even a hint as to the script's purpose.
      If caller doesn't explicitly pass a desc, at least shorten to just the
      basename of the executable.
      999484a6
    • Nat Goodspeed's avatar
      Make LLProcess post termination event to specified pump if desired. · 8b5d5f96
      Nat Goodspeed authored
      This way a caller need not spin on isRunning(); we can just listen for the
      requested termination event.
      Post a similar event containing error message if for any reason
      LLProcess::create() failed to launch the child.
      Add unit tests for both cases.
      8b5d5f96
  15. Feb 18, 2012
  16. Feb 16, 2012
    • Nat Goodspeed's avatar
      Fix bug in LLProcess::ReadPipe::peek() substring computation. · a06ba836
      Nat Goodspeed authored
      Add unit tests for peek() with substring args, reimplemented contains(),
      various forms of find().
      (yay unit tests)
      a06ba836
    • Nat Goodspeed's avatar
      Add LLProcess::ReadPipe::find() methods, with corresponding npos. · e92c3113
      Nat Goodspeed authored
      If it's useful to have contains() to tell you whether incoming data contains a
      particular substring, and if it's useful for contains() and peek() to accept
      an offset within that data, then it's useful to allow you to get the offset of
      a desired substring within that data. But of course a find() returning offset
      needs something like std::string::npos for "not found"; borrow that
      convention.
      Support both find(const std::string&) and find(char); the latter permits a
      more efficient implementation. In fact, make find(string) recognize a string
      of length 1 and leverage the find(char) implementation.
      Given that, reimplement contains(mumble) as shorthand for find(mumble) != npos.
      Implement find() overloads using std::search() and std::find() on
      boost::asio::streambuf character iterators, rather than copying to std::string
      and then using string search like previous contains() implementation.
      Reimplement WritePipeImpl::tick() and ReadPipeImpl::tick() to write/read
      directly from/to boost::asio::streambuf data, instead of copying to/from a
      temporary flat buffer.
      As long as ReadPipeImpl::tick() keeps successfully filling buffers, keep
      reading. Previous implementation would only handle a long child write over
      successive tick() calls. Stop on read error or when we come up short.
      e92c3113
  17. Feb 15, 2012
    • Nat Goodspeed's avatar
      Add LLProcess::ReadPipe::size(), peek(), contains(). · 56d93121
      Nat Goodspeed authored
      Also add "len" key to event data on LLProcess::getPump(). If you've used
      setLimit(), event["data"].length() may not reflect the length of the
      accumulated data in the ReadPipe.
      Add unit test with stdin/stdout handshake with child process.
      56d93121
    • Nat Goodspeed's avatar
    • Nat Goodspeed's avatar
      Preliminary pipe support for LLProcess. · e239cad1
      Nat Goodspeed authored
      Add LLProcess::FileParam to specify how to construct each child's standard
      file slot, with lots of comments about features designed but not yet
      implemented. The point is to design it with enough flexibility to be able to
      extend to foreseeable use cases.
      Add LLProcess::Params::files to collect up to 3 FileParam items. Naturally
      this extends the accepted LLSD syntax as well.
      Implement type="" (child inherits parent file descriptor) and "pipe" (parent
      constructs anonymous pipe to pass to child).
      Add LLProcess::FILESLOT enum, plus methods:
      getReadPipe(FILESLOT), getOptReadPipe(FILESLOT)
      getWritePipe(), getOptWritePipe()
      getPipeName(FILESLOT): placeholder implementation for now
      Add LLProcess::ReadPipe and WritePipe classes, as returned by get*Pipe().
      WritePipe supports get_ostream() method for streaming to child stdin.
      ReadPipe supports get_istream() method for reading from child stdout/stderr.
      It also provides getPump() returning LLEventPump& so interested parties can
      listen for arrival of new data on the aforementioned std::istream.
      For "pipe" slots, instantiate appropriate *Pipe class.
      ReadPipe and WritePipe classes are pure virtual bases for ReadPipeImpl and
      WritePipeImpl, respectively: all implementation data are hidden in the latter
      classes, visible only in llprocess.cpp. In fact each *PipeImpl class registers
      itself for "mainloop" ticks, attempting nonblocking I/O to the underlying
      apr_file_t on each tick. Data are buffered in a boost::asio::streambuf, which
      bridges between std::[io]stream and the APR I/O calls.
      Sanity-test ReadPipeImpl by using a pipe to absorb the Python "SyntaxError"
      output from the successful syntax_error test, rather than alarming the user.
      Add first few unit tests for validating FileParam. More tests coming!
      e239cad1
  18. Feb 13, 2012
    • Nat Goodspeed's avatar
      Use per-frame ticks on "mainloop" LLEventPump to update LLProcess. · aae61392
      Nat Goodspeed authored
      When we reimplemented LLProcess on APR, necessitating APR's funny callback
      mechanism to sense child-process status, every isRunning() or getStatus() call
      called the APR poll function that calls ALL registered LLProcess callbacks. In
      other words, every time any consumer called any LLProcess::isRunning() method,
      all LLProcess callbacks were redundantly fired. Change that so that the single
      APR poll function is called once per frame, courtesy of the "mainloop"
      LLEventPump. Once per viewer frame should be well within the realtime duration
      in which it's reasonable to expect child-process status to change.
      In effect, this changes LLProcess's public API to introduce a dependency on
      "mainloop" ticks. Add such ticks to llprocess_test.cpp as well.
      aae61392
    • Nat Goodspeed's avatar
  19. Feb 09, 2012
  20. Feb 07, 2012
    • Nat Goodspeed's avatar
    • Nat Goodspeed's avatar
      Convert LLProcess implementation from platform-specific to using APR. · aafb03b2
      Nat Goodspeed authored
      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.
      aafb03b2
  21. Jan 30, 2012
  22. Jan 27, 2012
    • Nat Goodspeed's avatar
      On Windows, only quote LLProcess arguments if they seem to need it. · 27df0a84
      Nat Goodspeed authored
      On Posix platforms, the OS argument mechanism makes quoting/reparsing
      unnecessary anyway, so this only affects Windows.
      Add optional 'triggers' parameter to LLStringUtils::quote() (default: space
      and double-quote). Only if the passed string contains a character in
      'triggers' will it be double-quoted.
      This is observed to fix a Windows-specific problem in which plugin child
      process would fail to start because it wasn't expecting a quoted number.
      Use LLStringUtils::quote() more consistently in LLProcess implementation for
      logging.
      27df0a84
  23. Jan 23, 2012
  24. Jan 22, 2012
  25. Jan 20, 2012
    • Nat Goodspeed's avatar
      Define LLProcess::Params; accept create(const LLSDParamAdapter<Params>&). · 6e214960
      Nat Goodspeed authored
      This allows callers to pass either LLSD formatted as before -- which all
      callers still do -- or an actual LLProcess::Params block.
      6e214960
    • Nat Goodspeed's avatar
      Per Richard, replace LLProcessLauncher with LLProcess. · f0dbb878
      Nat Goodspeed authored
      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.
      f0dbb878
Loading