Unblock subprocesses waiting for stdin close

[plockc]

Partial fix for #552

New method added _close_proc_stdin to allow the parent Runner to ask
subclass to close the stream after it finds the end of the stream. If the subclass
has not implemented _close_proc_stdin yet, then it is a no-op (and the bug
remains)

Fix ExceptionHandlingThread.is_dead which before would report dead when
thread was not alive and there was no exception info

This does not fix when pty=True, more significant changes are required for that case

Two test cases were adjusted that were wrongly asserting that the thread
should be "is_dead" after thread.join()

[plockc]

The appveyor seems to fail on many (all?) jobs with failing to find:
C:\Python27\Scripts\spec.exe

[plockc]

can check for the existence of attribute on self to allow subclasses to defer the fix

[bitprophet]

I'm a bit leery about the is_dead change since you're changing the test to just match the new behavior. Been a while since I looked at this part of the code, but I'm guessing the original author explicitly intended the is_dead flag to exhibit the properties it did in the test. Inverting the logic seems like a bad idea?

Can you provide more details about how the current test and implementation are both wrong? Thanks!

[plockc]

Hi @bitprophet I have to run but had time to answer some of your concerns, I can cover the two usage locations of is_dead usage later if you need:

Original code:

return (not self.is_alive()) and self.exc_info is not None

New code:

alive = self.is_alive() and (self.exc_info is None)
        return not alive

For new code, if a is "is_alive" and b is "self.exc_info is None", we can substitute and then rearrange for the is_dead calculation:

!alive
!(a and b) 
!a or !b  # demorgan's
!(is_alive) or !(self.exc_info is None)
not self.is_alive or self.exc_info is not None

Which is exactly the same as the original except using or instead of and

---

Consider the case where the thread completes normally, the thread's is_alive will be False, in the original code:

(not self.is_alive()) and self.exc_info is not None
(not False) and self.exc_info is not None
True and self.exc_info is not None
self.exc_info is not None

Which means: is_dead == has exc_info . . . but if we exit normally, there would be no exception captured, and so is_dead reports false, keeping the input thread alive

---

The test case was implemented incorrectly to make it pass, here is one of them:
https://github.com/plockc/invoke/blob/master/tests/concurrency.py#L39-L42

 t = EHThread(target=self.worker, args=[Queue()])
 t.start()
 t.join()
assert not t.is_dead

The last line is asserting that the thread is alive, right after a thread join, thread.join() docs: "Wait until the thread terminates"

[plockc]

i updated the tests for additional validation of underlying conditions, and the call to the new close stdin is now checked first to allow subclasses in other repos (fabric?) to update on it's own schedule

I think we'll want to squash the commmits, kept separate for review purposes

[plockc]

I didn't call it out here, but there is a minimal test scenario in #552

[plockc]

To make things more interesting, occassionally (maybe 1/4 of the time), I get an UnexpectedExit error in program.py:run (running the same task) when I have in_stream=StringIO(meta_data):

Encountered a bad command exit code!

Command: 'cd /var/lib/libvirt/images/base && sudo tee meta-data > /dev/null'

Exit code: None

Stdout: already printed

Stderr: already printed

I don't know yet if it's something I introduced, or pointing to something deeper and darker.

The Result is false, printing out the Result shows:

Command was not fully executed due to watcher error.
(no stdout)
(no stderr)

Seems like there was some handling for watchererror but that's not being activated by what result is showing, and not sure yet why/where there is a watcher error.

I think the process hasn't finished terminating / set the returncode, if i return poll() for returncode, I can't repro the issue . . not sure why we'd get past all the thread joins though while process is still alive though.

[seansfkelley]

This issue is making it impossible to write a task that feeds a string to the stdin when shelling out. I'm doing something along these lines:

@task
def t(c):
  input_string = ...
  c.run("program --that waits/until/stdin/ends", in_stream=StringIO(input_string))

The process just hangs forever, because the program won't do anything until stdin is exhausted.

A really simple repro:

from invoke import task
from io import StringIO

@task
def t(c):
    c.run("cat", in_stream=StringIO("foo"))

If you run this task, it should print 'foo' and then hang forever.

I tried all manner of combinations of closeing the stream before/after the call or writing control characters to the stream, but no dice.

@bitprophet what needs to be done to get this PR merged?

See also #575 (comment).

[shvechikov]

I'm having the same issue as @seansfkelley — just trying to send dynamic input to a process using in_stream=StringIO(...) parameter and it hangs.
@bitprophet Do we do something wrong?

[bitprophet]

I never closed this when 1.3.0 came out - it includes fixes for this including potentially some cherry-picks. Should be fixed now.

[brianbolt]

Still reproducible in fabric==2.7.1