Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PyDMShellCommand redirect output issues with no tty #1039

Open
ZLLentz opened this issue Oct 3, 2023 · 2 comments
Open

PyDMShellCommand redirect output issues with no tty #1039

ZLLentz opened this issue Oct 3, 2023 · 2 comments

Comments

@ZLLentz
Copy link
Member

ZLLentz commented Oct 3, 2023

Describe the bug

I'm willing to implement a fix here, but I'd like some discussion/guidance first.

I noticed that some of our shell command buttons would "stop working", because our scientists reported that our applications wouldn't launch. I spent some time digging into it and figured out the following:

  • The PyDMShellCommand button has a redirectCommandOutput property that, when changed to True, will send the stdout from the encapsulated process to the stdout of the process that contains the button. (When left at False, this output will be dropped instead).
  • In some rare cases (see "Steps to Reproduce" below), the terminal that launched the pydm process can be closed without killing the pydm process.
  • In these cases, there is no longer a terminal for the stdout to be sent to. If you check the process list with ps, you'll notice that both the pydm process and the subprocess no longer have a TTY assigned to them.
  • If the subprocess is also a Python process, and only in some of these cases (I haven't figured out why this only happens sometimes), this causes the internal process to raise an OSError (<class 'OSError'>: [Errno 5] Input/output error) when trying to print text to the screen.
  • This causes the internal process raised from the command button to crash, and if this crash happens on startup we all get very confused.

Expected behavior

  • I expect PyDM to handle the subprocess stdout in a way that the process can continue running, even if the source terminal is lost.

Steps to Reproduce

  1. Create a python script that prints something and then also does some non-printing action you can check later, such as writing to a file or opening a gui. (Note: I haven't fully figured out which kinds of scripts here trigger the issue, but pydm guis themselves as subprocesses seem to be the most common offenders.)
  2. Create a ui file that contains a PyDMShellCommand that runs your script with redirectCommandOutput set to True (checked)
  3. Create a launcher script to help start and orphan the pydm process. It should be a shell script and contain a line like pydm my_file.ui &
  4. Open a new terminal on the machine that will run the pydm process. This can either done locally while sitting at the computer or remotely using e.g. xterm to launch a local terminal in a new window.
  5. In this new terminal, run your launcher script
  6. Close the terminal once the screen has appeared
  7. Click the button and verify that the non-printing action did not complete

Possible Solution

I had a few things in mind but I want other opinions:

  • Redirect the subprocess stdout to a function we control, rather than directly to stdout, so we can do some error handling
  • Rework this widget to always store the output locally in a way that we can check at runtime (separate from other terminal output)

My Platform

OS red hat 7
pydm v1.19.1
python v3.9.15
pyqt v5.12.3

Additional context

  • This was originally found with an application that crashes so poorly we thought everything was freezing.
  • We found this a while ago but had a lot of trouble reproducing/understanding the bug to the point where I could make a coherent issue
@jbellister-slac
Copy link
Collaborator

Thanks for digging into this and managing to figure all that out! I should have a chance to reproduce this near the end of today so I can fully understand, and can let you know what I think afterwards.

@ZLLentz
Copy link
Member Author

ZLLentz commented Oct 5, 2023

Thank you! Note that, locally, we're avoiding this now by "not doing that" e.g. not redirecting the print output to the terminal, so I don't consider this to be an "urgent" edge case per se. It can also be avoided by changing the way we launch our screens. But in general, it felt like something we could also do something about on the PyDM side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants