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

SaltStack (using Python subprocess) hangs when running php-fpm 8.1.11 #9754

Closed
bgdnlp opened this issue Oct 14, 2022 · 7 comments
Closed

SaltStack (using Python subprocess) hangs when running php-fpm 8.1.11 #9754

bgdnlp opened this issue Oct 14, 2022 · 7 comments
Assignees

Comments

@bgdnlp
Copy link

bgdnlp commented Oct 14, 2022

Description

I am cross-posting saltstack/salt#62891 because I'm not sure what's happening, but it is related to PHP 8.1.11 and 8.0.24, previous versions worked fine.

I tested on Ubuntu with Python 3.10 and FreeBSD with Python 3.9. Running

salt-call -l all cmd.run 'php-fpm8.1'

on Ubuntu or the equivalent on FreeBSD just hangs. It didn't use to do that.

To reproduce:

  • start an Ubuntu instance
  • install PHP 8.1:
    • apt-get install software-properties-common
    • sudo apt-get install software-properties-common
    • sudo add-apt-repository ppa:ondrej/php
    • sudo apt-get update
    • sudo apt install php8.1-fpm
  • install SaltStack:
    • sudo apt install python3-pip
    • sudo pip3 install --upgrade salt
    • sudo salt-minion <- run once. it will set up /etc, then it's not needed any more
  • run some random command with SaltStack, just to see how it should work:
    • sudo salt-call -l all cmd.run 'ls'
  • run php-fpm with SaltStack, this should just hang, although php-fpm does start:
    • sudo salt-call -l all cmd.run 'php-fpm8.1'

SaltStack hangs when it gets to this line: https://github.com/saltstack/salt/blob/427718c5aed40b376785ebc4813f5ccd18e07272/salt/utils/timed_subprocess.py#L90 . At that point php-fpm is already running, Salt is just trying to interact with php-fpm.

I wasn't able to reproduce in Python directly.

PHP Version

PHP 8.1.11 and PHP 8.0.24

Operating System

FreeBSD 13.1 and Ubuntu 22.04

@bukka
Copy link
Member

bukka commented Oct 21, 2022

I think you might want to run FPM in foreground in this case so maybe try to run FPM as php-fpm8.1 -F (basically just add -F option to run it in foreground). If you need it run as a root, you might also need to add -R option... See php-fpm8.1 --help for more details.

@bgdnlp
Copy link
Author

bgdnlp commented Oct 21, 2022

I think you might want to run FPM in foreground

That's not an option, what I'm actually trying to do is start the php-fpm service, which means -D. That's the actual problem, I can no longer set up PHP through config management. The cmd.run line is just the easiest way to reproduce that I could find. If I ran it in the foreground, Salt would still hang, but then I wouldn't know if it is because of the bug, or because it's just waiting for the process to exit.

There are some other details in the related issue I opened in SaltStack repo.

@bukka
Copy link
Member

bukka commented Oct 22, 2022

Ok this might need some deeper investigation and might also need more looking to the SaltStack integration. I added that to my big TODO list but there are bunch of other higher priority tasks in front of it so it will likely take some time to get into it.

@bukka
Copy link
Member

bukka commented Oct 22, 2022

Actually I put a bit higher on my TODO list in case it's a regression. It shouldn't hopefully take that much time to look into it. So should be hopefully able to look more in the next few weeks.

@bukka bukka self-assigned this Oct 28, 2022
@bukka
Copy link
Member

bukka commented Oct 28, 2022

Just a quick update that I spent some time on this today and did some debugging. I managed to recreate the issue under the Salt stack and then extract it to the subprocess call that hangs with PHP-8.0 but works with PHP-7.4:

import subprocess

kwargs = {
    'stdout': subprocess.PIPE,
    'stderr': subprocess.STDOUT,
    'executable': '/bin/bash',
    'shell': True,
}
proc = subprocess.Popen(['php-fpm8.0'], **kwargs)
out, err = proc.communicate()
print(out, err)

Now I will need to figure out what changed in 8.0 and why it's not working.

@bgdnlp
Copy link
Author

bgdnlp commented Oct 29, 2022

It worked with the previous minor version of PHP too. PHP 8.1.10 didn't have this problem. Should help narrow it down.

bukka added a commit to bukka/php-src that referenced this issue Oct 29, 2022
SaltStack uses Python subprocess and redirects stderr to stdout which is
then piped to the returned output. If php-fpm starts in daemonized mode,
it should close stderr. However a fix introduced in phpGH-8913 keeps stderr
around so it can be later restored. That causes the issue reported in
phpGH-9754. The solution is to keep stderr around only when php-fpm runs in
foreground as the issue is most likely visible only there. Basically
there is no need to restore stderr when php-fpm is daemonized.
bukka added a commit to bukka/php-src that referenced this issue Oct 29, 2022
SaltStack uses Python subprocess and redirects stderr to stdout which is
then piped to the returned output. If php-fpm starts in daemonized mode,
it should close stderr. However a fix introduced in phpGH-8913 keeps stderr
around so it can be later restored. That causes the issue reported in
phpGH-9754. The solution is to keep stderr around only when php-fpm runs in
foreground as the issue is most likely visible only there. Basically
there is no need to restore stderr when php-fpm is daemonized.
bukka added a commit to bukka/php-src that referenced this issue Oct 29, 2022
SaltStack uses Python subprocess and redirects stderr to stdout which is
then piped to the returned output. If php-fpm starts in daemonized mode,
it should close stderr. However a fix introduced in phpGH-8913 keeps stderr
around so it can be later restored. That causes the issue reported in
phpGH-9754. The solution is to keep stderr around only when php-fpm runs in
foreground as the issue is most likely visible only there. Basically
there is no need to restore stderr when php-fpm is daemonized.
bukka added a commit to bukka/php-src that referenced this issue Oct 29, 2022
SaltStack uses Python subprocess and redirects stderr to stdout which is
then piped to the returned output. If php-fpm starts in daemonized mode,
it should close stderr. However a fix introduced in phpGH-8913 keeps stderr
around so it can be later restored. That causes the issue reported in
phpGH-9754. The solution is to keep stderr around only when php-fpm runs in
foreground as the issue is most likely visible only there. Basically
there is no need to restore stderr when php-fpm is daemonized.
@bukka
Copy link
Member

bukka commented Oct 29, 2022

Thanks that helped me to find the cause a bit quicker - the fix is in #9852 and should be hopefully part of PHP 8.0.26 and 8.1.13. I managed to put together a test for this so it won't hopefully happen again.

@bukka bukka closed this as completed in 1c5844a Oct 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants
@bukka @bgdnlp and others