php / php-src

The PHP Interpreter
https://www.php.net
Other
38.14k stars 7.74k forks source link

macOS now crashes fork() process instead of just warning output #11818

Open askdkc opened 1 year ago

askdkc commented 1 year ago

Description

The latest macOS update has begun terminating certain fork() processes under specific circumstances. This is evidenced by the following screenshots and error logs

255328277-612559bd-d64e-4231-a6c5-5a60c9ba703a

In the Nginx Error Log, we find multiple instances of "upstream prematurely closed connection" errors. The PHP Error Log indicates warnings about "__NSPlaceholderDate initialize" possibly running in another thread when fork() was invoked, and subsequently, child processes exiting prematurely.

How to reproduce

Follow this step: https://github.com/laravel/valet/issues/1433#issuecomment-1653063304

Work Around (so far)

Rails users faced a similar issue a few years ago (refer to rails/rails#38560) and resolved it by introducing the environment variable

export OBJC_DISABLE_INITIALIZE_FORK_SAFETY=YES.

macOS php installed using Homebrew can achieve this by adding

    <key>EnvironmentVariables</key>
    <dict>
        <key>OBJC_DISABLE_INITIALIZE_FORK_SAFETY</key>
        <string>YES</string>
    </dict>

to PHP LauchDaemon plist homebrew.mxcl.php.plist. But ultimately it should be fixed in PHP itself.

PHP Version

PHP 8.2.8

Operating System

macOS 13.5

For People using Homebrew PHP

I've done much troubleshooting on this problem in Homebrew Issue (https://github.com/Homebrew/homebrew-core/issues/137431). If you have problems with your Homebrew PHP, please refer to it๐Ÿ˜‰

carlocab commented 1 year ago

See https://blog.phusion.nl/2017/10/13/why-ruby-app-servers-break-on-macos-high-sierra-and-what-can-be-done-about-it/ for additional background.

askdkc commented 1 year ago

@carlocab

I understand that the ideal solution would be to fix this in the main PHP source code. However, that could take some time, and meanwhile, all developers using macOS will continue to encounter this issue.

Incorporating this workaround into Homebrew's PHP could serve as a good interim solution. Without it, many people would have to spend unnecessary time searching and digging for solutions, which isn't really efficient or helpful, don't you think?

carlocab commented 1 year ago

I'm open to incorporating a workaround for Homebrew installations of PHP, but I'm just not convinced that setting OBJC_DISABLE_INITIALIZE_FORK_SAFETY is appropriate or safe to do, especially for users who are unaware that we've adopted this workaround.

askdkc commented 1 year ago

Iโ€™m agreeing with you.

I mean if this is happening to all the platforms like Linux and Windows, I donโ€™t recommend this setting either.

But itโ€™s kinda Apple who suddenly killing php fork() process behind the scenes in this case, I guess. Which is also unaware of for many of us until things start getting broken like this.

Setting OBJC_DISABLE_INITIALIZE_FORK_SAFETY is not like adding a new thing. It is more like backward compatibility in this case.

askdkc commented 1 year ago

The log suggests, when certain Objective-C methods are in progress in another thread when fork() is called, the child process will now crash.

This is more of a system-level issue than a PHP issue, which makes it somewhat difficult to handle within the PHP code itself. If you're using PHP-FPM with Nginx on macOS, a potential workaround is to switch to using PHP-CGI, but that's not helpful solution.

Apple's decision to kill these processes means that certain PHP features, like PHP-FPM, are now somewhat inaccessible to many macOS developers without using the OBJC_DISABLE_INITIALIZE_FORK_SAFETY=YES workaround.

devnexen commented 1 year ago

This is more of a system-level issue than a PHP issue.

It is indeed, disabling at boot time the System Integrity Protection might help a bit (among other things it allows you to full dtrace capacities) but that's not something I would recommend except on a VM perhaps. Most of people facing this issue recommend moving on towards posix_spawn.

askdkc commented 1 year ago

@devnexen

Thank you for checking this.

What would you suggest to people who are using Homebrew PHP on their Mac?

I'm using Laravel and Laravel Valet, and so far, this workaround (https://github.com/laravel/valet/issues/1433#issuecomment-1653419658) works for me, and many others.

Should Homebrew include this fix, https://github.com/Homebrew/homebrew-core/pull/137909, for the time being?

devnexen commented 1 year ago

Should Homebrew include this fix, Homebrew/homebrew-core#137909, for the time being?

It is certainly handier if homebrew included it, now I m not sure about "should" tough, there is more chance they refuse to merge it than the other way around. I ll start to have a look at it in the following days, let's face it fork on macos is kind of "broken" no matter what.

askdkc commented 1 year ago

@devnexen

Thanks!

So far, the workaround does work in many cases. But if you can manage to get it fixed, that'll be amazing ๐Ÿ‘

https://github.com/Homebrew/homebrew-core/issues/137431

https://github.com/laravel/valet/issues/1433

For the "should" part, Homebrew is a package manager, like apt and def(yum) for some Linux distros. People using package manager don't expect broken changes. If their services get broken after they perform the update using their package manager, the package manager's job should quickly release fixed version (or perhaps includes workaround version). That's what my "should" part means.

devnexen commented 1 year ago

@devnexen

Thanks!

So far, the workaround does work in many cases. But if you can manage to get it fixed, that'll be amazing ๐Ÿ‘

Would putting this env var in .zshrc/.bash_profile work as well ?

askdkc commented 1 year ago

Would putting this env var in .zshrc/.bash_profile work as well ?

Nope. Unfortunately ๐Ÿ˜•

drbyte commented 1 year ago

Would putting this env var in .zshrc/.bash_profile work as well ?

As @askdkc said, no it won't.

That's because php-fpm isn't normally instantiated from a shell command. If it were, then yes it might work ... but then you'd have to make sure you know what kind of shell is active and add it to that shell's profile config. Not an ideal bandage.

devnexen commented 1 year ago

Would putting this env var in .zshrc/.bash_profile work as well ?

Nope. Unfortunately ๐Ÿ˜•

Would you be able to build php from sources ? if yes could try the following PR to see if it fixes. If it does not, I m afraid, the homebrew's change would be the only resort.

askdkc commented 1 year ago

@devnexen

I tried. I finally passed ./configure part.

But running make throws a following error.

Src/php-src/ext/ffi/ffi.c:29:10: fatal error: 'ffi.h' file not found
#include <ffi.h>
         ^~~~~~~
1 error generated.
make: *** [ext/ffi/ffi.lo] Error 1

@carlocab Could you try and test @devnexen fix?

askdkc commented 1 year ago

@devnexen

OK, I've done some googling and chatGPTing and fixed some missing env parameters for the build process. And I could finally get it installed.

But result is the same. 502 error.

And adding following values to homebrew.mxcl.php.plist still fix the error.

<key>EnvironmentVariables</key>
<dict>
    <key>OBJC_DISABLE_INITIALIZE_FORK_SAFETY</key>
    <string>YES</string>
</dict>

So your solution(https://github.com/php/php-src/pull/11895), by adding following line to configure.ac doesn't work, unfortunately ๐Ÿ˜ข

  *darwin*)
    PHP_ADD_FRAMEWORK(Foundation)
    ;;

@carlocab

I think we've done so much researching on this. Could you merge this PR (https://github.com/Homebrew/homebrew-core/pull/137909) for the time being? We can always revisit it for further improvements in the future if needed, right?

carlocab commented 1 year ago

Setting OBJC_DISABLE_INITIALIZE_FORK_SAFETY is not like adding a new thing. It is more like backward compatibility in this case.

Sure, but, as mentioned in the link I shared above, backward compatibility is not safe.

Before High Sierra: appearing to work most of the time, occasionally failing catastrophically.

Most of the time, you're lucky and no other threads are doing important stuff. The application will appear to work fine. There may still be corrupted state somewhere, but you don't notice it โ€” at least not immediately.

If you're unlucky then the application crashes or freezes in mysterious ways. In theory it can even accidentally wipe your hard drive.

Takeaway: the problem has always been there, even before High Sierra and even on Linux. It's just that High Sierra has chosen to fail fast instead of silently allowing corruption.

askdkc commented 1 year ago

@carlocab

This is macOS we are talking about which doesn't come with PHP.

Only Devs are using Homebrew to install PHP because they need it. If something terrible happens, that's because the dev wrote bad code. That should be their responsibility to worry about. Preventing it from happening is nice as long as users are just users, not devs.

Homebrew doesn't include the workaround is like making a Lamborghini with 40km/h speed limits on. Maybe safe, but useless to many.

PhilippImhof commented 1 year ago

Are there any news on this? For me, the workaround with OBJC_DISABLE_INITIALIZE_FORK_SAFETY does not work, so dev with PHP has become very difficult on my machine...

coreysmithdesign commented 1 year ago

Saw this reference to it earlier today. I'm hoping it's some progress?

https://github.com/Homebrew/homebrew-core/issues/137431#issuecomment-1691299181

In the meantime, Herd's precompiled PHP has been working for me until this is resolved. I just pointed it at my project directory.

bukka commented 1 year ago

So if I understand it correctly, this is emitted by the child process so it might do to just set it in FPM master process before any forking is done. Can anyone check if #12044 fixes the issue?

askdkc commented 1 year ago

@bukka

It's funny to see that you implementing the fix that Homebrew guys rejected.

bukka commented 1 year ago

I'm not really sure what we could do otherwise. FPM cannot work without fork. I have been thinking about posix_spawn but it is just not viable at this stage as we would basically have to redesign many things - certainly not possible to use it as a bug fix and it will unlikely be used anytime soon in the next version as there are higher priorities atm.

We should also note that PHP is mostly used for development on Mac. We can probably add a configuration option to disable this but I would be probably in favour to leave it on be default if this will make things work.

Of course if anyone can think about better solution, then I would be more than happy to use that instead.

askdkc commented 1 year ago

@bukka

I totally agree with you.

I mean, I created this PR https://github.com/Homebrew/homebrew-core/pull/137909 because this is macOS only problem and many of us use Homebrew to install php on macOS.

If Homebrew doesn't like my workaround idea, but PHP team likes it? Well then, just go for it. Without this fix, it just keeps people using macOS crazy.

bukka commented 1 year ago

@askdkc Ok, are you able to test it and confirm that it works?

askdkc commented 1 year ago

@bukka

give me few days. I will see if I can test this (or not).

NattyNarwhal commented 1 year ago

The right solution is probably move anything that touches Objective-C classes ahead of time (which, looks like date and probably locale per the stack traces I've seen - if you can get a stack trace, even better) in global init, before any forking. As previously mentioned, that environment variable isn't a real solution, it's disabling the safety abort in an unsound situation.

NattyNarwhal commented 1 year ago

What I'd like to have is a sample that could reproduce it reliably outside of a forking webserver. You'd think pcntl_fork would be adequate - the problem is determining what'd initialize Objective-C classes. I have this stupid bit of code (cobbling from stuff I'd think would poke Objective-C initializers), but no crash yet:

<?php

if (false !== setlocale(LC_ALL, 'en_CA.UTF-8')) {
    $locale_info = localeconv();
    print_r($locale_info);
}

$pid = pcntl_fork();
if ($pid == -1) {
     die('could not fork');
} else if ($pid) {
     pcntl_wait($status); // wait on child
} else {
        echo "child process\n";

        var_dump(date_parse("2006-12-12 10:00:00.5"));

        echo date("M d Y H:i:s", mktime(0, 0, 0, 1, 1, 1998)) . "\n";
        echo gmdate("M d Y H:i:s", mktime(0, 0, 0, 1, 1, 1998)) . "\n";
}
milhouse1337 commented 1 year ago

@NattyNarwhal as referenced here, you can crash it by using the gettext module (with FPM not the CLI). https://github.com/Homebrew/homebrew-core/issues/137431

Here is a test.

<?php
echo gettext("hello");

No bug if you fix your locale like so.

<?php
putenv('LC_ALL=en_US');
echo gettext("hello");

Here are the logs from php-fpm.log.

[25-Aug-2023 15:49:20] WARNING: [pool www] child 32668 exited on signal 11 (SIGSEGV) after 0.651017 seconds from start
[25-Aug-2023 15:49:20] NOTICE: [pool www] child 32672 started
[25-Aug-2023 15:49:20] WARNING: [pool www] child 32669 exited on signal 11 (SIGSEGV) after 0.495230 seconds from start
[25-Aug-2023 15:49:20] NOTICE: [pool www] child 32673 started

This bug is driving me crazy. ๐Ÿ˜…

PDO (dblib) also seems to be affected, you can follow this thread too. https://github.com/laravel/valet/issues/1433

bukka commented 1 year ago

So I have doing a bit more research and I am not sure we should do the work around that I proposed initially. The reason is that this is most likely related to the code changes in couple of libraries. So it might be actually better to try to identify those places and potentially try to fix the libraries if the problem is there. I will try to do some debugging on my mac and see if I get anywhere.

bukka commented 1 year ago

So it seems to me that the reported pg and potentially MongoDB are because of krb update: https://github.com/krb5/krb5/pull/1221 so it should be handled there in the first place.

Then there's a gettext that might need some debugging to find an exact place.

askdkc commented 1 year ago

So it seems to me that the reported pg and potentially MongoDB are because of krb update: krb5/krb5#1221 so it should be handled there in the first place.

If it's krb related, it should have started a lot earlier, don't you think? But this problem starts happening like after release of macOS 13.5.

Some say it might be related with Openssl 3 https://github.com/laravel/valet/issues/1433#issuecomment-1655992333.

In fact, mongodb starts working fine after building it with openssl extension https://github.com/laravel/valet/issues/1433#issuecomment-1665396558.

@bukka If you can nail this, it'll be fantastic๐Ÿ˜‰

RawkBob commented 1 year ago

Not sure how much help this is but I get a similar error when calling dns_get_record() along with gettext's _('something') when running on OS X 11.7.9, php 8.2.9, apache (httpd) 2.4.57.

httpd error log shows AH00052: child pid 5266 exit signal Segmentation fault (11)

Running the built in php (php -S) server works fine without any 'fixes', as does php artisan serve when using laravel.

The fork safety 'fix' does not make php/apache work, neither does running php/apache using sudo. Using the gettext fix does work, for gettext only, but dns_get_record() still crashes.

Disabling fork safety on some installs just hides the underlying problem and isn't a valid fix

NattyNarwhal commented 1 year ago

I think the two things confusing me are:

  1. What APIs are using Objective-C behind the scenes? And how? The low-level stuff in libSystem likely isn't. gettext seems to be, but I can't imagine how; I doubt it's calling into Foundation et al.
  2. If this is some kind of forking issue, it should be easily reproducible by calling an objc using API, forking via pcntl, and calling another one from the child. (The reason I'd like to do that is to separate it from FPM, to make it easier to debug.) Is FPM aggrevating this by calling some objc API on its own init?
PhilippImhof commented 1 year ago

@RawkBob

Just a guess from the error message: are you using mod_php rather than php-fpm? In that case, you could try using SetEnv in httpd.conf.

Also, I had some crashes that went away after updating xdebug.

drbyte commented 1 year ago

@PhilippImhof

Also, I had some crashes that went away after updating xdebug.

Do you recall what version of xdebug you had before? vs after? Was xdebug installed via pecl? If so, that might have been a recompile with dependency libs already on your Mac.

PhilippImhof commented 1 year ago

I went from 3.2.1 to 3.2.2 and used pecl.

RawkBob commented 1 year ago

Just a guess from the error message: are you using mod_php rather than php-fpm? In that case, you could try using SetEnv in httpd.conf.

I was running as mod_php, switching to php-fpm makes dns_get_record() work, but gettext still needs the putenv('LC=C') to be set... still not fixing the underlying issue

greghudson commented 1 year ago

So it seems to me that the reported pg and potentially MongoDB are because of krb update: krb5/krb5#1221 so it should be handled there in the first place.

(Primary MIT krb5 maintainer here.) My current understanding is:

[1] https://github.com/Homebrew/homebrew-core/pull/47494#issuecomment-562400120 [2] https://github.com/Homebrew/homebrew-core/pull/47494#issue-532907622

bukka commented 1 year ago

@greghudson Thanks for the comment. In terms of PHP-FPM, it should be just forking while still single-threaded. Would it be also possible to somehow disable credential cache from client library?

I have been thinking about this and essentially we need to somehow change the config if it is used by PHP in the forked process. It means to somehow change configuration during runtime. It might not be that easy to do it in krb5 because PHP uses that through libpq so we might initially just make sure that gssapi auth is not used if we can't change cred cache in krb5 through it. So the first step would be to check libpq and see what options are there for auth and possibly request some changes there.

@devnexen as you maintain pgsql ext, do you know if there are some options in libpq for the above?

bukka commented 1 year ago

@devnexen So just found that in libpq could disable it GSSAPI using connection parameter: https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-GSSENCMODE . So if we can change krb5 credential cache, we could maybe introduce some easy way for users to disable / enable it and have it disabled by default on Mac.

Or maybe brew could just set PGGSSENCMODE=disable when starting FPM. @carlocab What do you think about that?

morrisonlevi commented 1 year ago

I loathe fork without exec. I wish everyone stopped using it for anything. These issues that Mac OS are now surfacing have essentially always been there. They are taking them more seriously than other platforms.

However, I'm not sure what Mac is doing is actually correct. Can anyone who is experiencing these issues confirm whether any threads have been made prior to the fork? If not, then these warnings and errors are wrong. These are only issues if there is a fork in the presence of threads. Spawning threads in the child after fork should be fine, as long as that process never forks.

That, or I have misunderstood the issue. While that is certainly possible because these things can be really subtle sometimes, I'm more inclined to think that:

  1. Something is being naughty and spawning threads before the pre-fork.
  2. Mac is just wrong.

If anyone has the bandwidth to get php-fpm to stop forking and to instead use one of the newer methods to make child processes, that would also be great, but understanding what exactly is wrong with the above is still important.

NattyNarwhal commented 1 year ago

Looks like things might have changed in macOS 14.

MrMage commented 1 year ago

Looks like things might have changed in macOS 14.

@NattyNarwhal what exactly has changed? The diff is fairly large; would be great to know what the relevant part is.

bukka commented 1 year ago

If anyone has the bandwidth to get php-fpm to stop forking and to instead use one of the newer methods to make child processes, that would also be great

This might be quite a big task and would require significant change in the whole FPM inner working as it impacts the whole master / child communication (mainly scoreboard), how childrens are currently managed in master, the shared state and potentially some other bits. I'm not saying it is not possible but I would not recommend this to anyone without a detailed FPM knowledge. Such task really requires good knowledge and some experience with FPM code.

bukka commented 1 year ago

I forgot to also mention one really important thing that is really hard to overcome. It's the fact that MINIT is done by master and all globals initialization is done there and then shared by children after fork. To make it work with exec we would need to move MINIT to child but that creates problems for extension using shared resources like opcache so all of those would need to adapted to the new model. We would most likely need a new module hooks and do lots refactoring. As I said, it is quite a big task...

cbandy commented 11 months ago

Looks like things might have changed in macOS 14.

@NattyNarwhal what exactly has changed? The diff is fairly large; would be great to know what the relevant part is.

@MrMage I believe this is linking to the bottom hunk in runtime/objc-initialize.mm of this commit.