Process.__repr__ displays stale information

[dhirschfeld]

As discussed on gitter Process.__repr__ doesn't show the latest information:

In [28]: process = await trio.open_process(['git', 'status'], stdout=subprocess.DEVNULL)

In [29]: process
Out[29]: <trio.Process ['git', 'status']: running with PID 9888>

In [30]: !powershell -Command { 'ps -Id 9888' }
ps : Cannot find a process with the process identifier 9888.
At line:1 char:1
+ ps -Id 9888
+ ~~~~~~~~~~~
    + CategoryInfo          : ObjectNotFound: (9888:Int32) [Get-Process], ProcessCommandException
    + FullyQualifiedErrorId : NoProcessFoundForGivenId,Microsoft.PowerShell.Commands.GetProcessCommand


In [31]: process
Out[31]: <trio.Process ['git', 'status']: running with PID 9888>

In [32]: await process.wait()
Out[32]: 0

In [33]: process
Out[33]: <trio.Process ['git', 'status']: exited with status 0>

Just opening this issue in case the situation can be improved by e.g. calling poll internally:

maybe __repr__ should automatically call .poll() to give up to date info?

[njsmith]

This is a weird thing about subprocess.Popen ... it has a separate .returncode attribute and .poll() method. Calling .poll() checks if the process has finished, and if so, it returns the value, and it also updates .returncode as a side-effect. .returncode OTOH is totally passive; it returns stale data until you explicitly call .poll() or .wait().

In this case, __repr__ here is using .returncode, so it can show stale data. The other option would be to use .poll(), so it would return up-to-date data... but then this would also mutate the .returncode attribute as a side-effect, and you usually don't want __repr__ to mutate anything.

So I sort of think we should instead fix .returncode, so that it's always up-to-date:

@property
def returncode(self):
    return self.poll()

And then __repr__ would show up-to-date information, and wouldn't have any user-visible mutation effects.