Using Php::out with Apache crashes

Hi,

i'm using Apache and PHP and I noticed that when I use:
Php::out << content << flush();
and do multiple page requests the process will be terminated with this error: "AH00428: Parent: child process 2512 exited with status 255 -- Restarting."

My current solution for this problem is a std::mutex around it but then i do requests more or less sequentially and not simultaneously :(

Basically what I want to do is to generate the page with header depending on the current request and return it to the client:

// Set the headers
const size_t headerSize = headers.size();
for (size_t i = 0; i < headerSize; ++i)
Php::call("header", headers[i]); // btw: is there any better way to set the header ?

// Set the content
Php::out << content;
Php::out.flush();

My Setting:
Windows 7(x64), Apache 2.4.9(x86), Php 5.5.12ts(x86) and Visual Studio 2013.

[EmielBruijntjes]

This is indeed a bug. The streams are not thread safe. I will look into this to see how we can make them thread safe, while we keep the same simple API.

[valmat]

btw: is there any better way to set the header ?

I just looked at the php source. It seems Zend API does not provide any tools to send headers.
The only place where I found the definition for sending headers -- this is /path/to/php-src/main/SAPI.h.

SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg TSRMLS_DC);
//...
SAPI_API int sapi_send_headers(TSRMLS_D);
//...
//etc

So the current implementation of PHP-CPP the only way to send the headers it Php::call("header", headers[i])
Function header() is seldom used, and if used, is not called many times. So I do not think it advisable to implement in PHP-CPP native support for header (). But if someone wants to do it, then all he need to do can be found in SAPI.h.

@EmielBruijntjes: Thx would be awesome if you could do that 👍 BIG THX :D
@valmat: Thx for the info and effort to looking into the php sources 👍 i will try do implement the sapi calls and check if they are working for me :D

[EmielBruijntjes]

A Php::header() function could be added, that directly calls the underlying C function to send the header. This would be faster.

What do you think about this bug Valmat? In C++ the std::ostreams are not thread safe themselves. It is therefore pretty hard to make a thread safe Php::out because we simply rely on std::ostream.

[valmat]

What do you think about this bug Valmat?

It is difficult for me to answer this question, because I have no experience of solving such tasks.
At a glance I can see two ways:

1. To use locks (very slowly).
2. To move the block

/**
 *  Create the actual steams
 *  @var std::ostream
 */
std::ostream out        (&bufOut);
std::ostream error      (&bufError);
std::ostream warning    (&bufWarning);
std::ostream notice     (&bufNotice);
std::ostream deprecated (&bufDeprecated);

so that would be out, error, etc. were created in a separate thread (perhaps this phrase makes no sense?).
By the way, I'm using nginx+php-fpm and I just checked. I could not find such problems.

As far as I know, the Apache implements three multiprocessing models: Worker, PreFork and Event. Probably the decision will depend on a model.

Some time ago I tested as global objects shared among threads.
I was not able to create a object shared to threads (php-fpm).

In summary:

1. Memory sharing between threads should depend on the multiprocessing model.
2. Perhaps matter at what stage of memory is allocated. I mean extension.onStartup([]() {...}); and extension.onRequest([]() {...});

All of the above is just my hasty assumptions.

On my opinion to solve this problem with the multi thread model you "only" have to use the thread local storage for the member variable StreamBuf::_buffer.
With the new and nice C++11 feature -> "thread_local char _buffer[1024];" you could do that but this will not work with the Visual Studio compiler >.< you only can use "__declspec(thread)" but this require to define the variable static like: "__declspec(thread) static char _buffer[1024];" and thats not working if you have multiple instances of StreamBuf >.<. Mabe someone knows either how to use the keyword thread_local with the MSVC or to solve this problem another way ? I keep further looking into this and try to find a solution. I'll keep you updated.

[EmielBruijntjes]

Hmm, I did not know about the 'thread_local' keyword. Nice.

The error that I made was that global variables are not allowed in a multi-threading PHP environment. The solution would be to change the StreamBuf class to not have a _buffer member variable, but to put the various _buffer variables inside the ZEND_BEGIN_MODULE_GLOBALS and ZEND_END_MODULE_GLOBALS.

I will look into this.

yeah it is :D
have you looked into this or can I help you with anything ?

[EmielBruijntjes]

I have been looking into this, and tried an implementation with the buffer being created inside the ZEND_*_MODULE_GLOBALS code. But my first attempt to implement it, and keep the same easy API, failed. It was not as easy to solve as I hoped it would be.

This all makes me doubt.

The point is, even in 'normal' C++ stream buffers (like std::cout) are not thread safe, and it is up to the programmer to be aware of this, and use the buffers with care. So one could argue, that - because PHP-CPP is just a normal C++ library - that it is also up to the programmer in this specific situation to deal with the streams not being thread safe. After all, that is the normal C++ stream behavior. Seen from this standpoint, this issue is not a bug after all.

However! When you're writing a PHP-CPP extension, you (as a programmer) do normally not have the feeling that you're working in a multi-threading environment, and you're allowed to call all other PHP-CPP functions without having to worry about thread safety at all. You could also argue that this should therefore also be true for the PHP-CPP streams, and that they should be thread safe. The fact that the server sometimes crashes when running in a multi-threading setup is thus a bug.

This bug is indeed really simple to solve by simply adding this thread_local keyword to the stream buffers. But the frustrating issue is, that this 'thread_local' keyword does not work on Windows environments - and it is exactly the Windows environments that mostly need this fix, as they seem to make more use of multi-threading PHP installations.

If you have ideas or a pull request, it would be very welcome. I really hope that you can keep the same simple API as we already have.

Thanks for looking into it and your detailed explanation :D

I'm completly unstand that the API should be as it is. Furthermure I completly agree that the code behind should enshure thread safty not the user of the API.

I think the issue can be solved if we swap Php::out's std::ostream with a class which has the stream operator overloaded. So basically it has the same look and feel of the std::ostream but on access it get's the current stream or buffer from the thread local storage. This would be a fast and a none blocking method to enshure thread safety.

What do you think ?

[weltling]

@martijnotto the commit b8a4b5f makes things not portable. Visual studio linker was never able to DLL export thread local data directly. Effectively thread_local __declspec(dllexport) puts two incompatible storage specifiers which Visual Studio won't even compile, see include/streams.h. Even leaving Visual Studio aside - thread_local should be automatically static, as by the standard.

An object like Php::out, that still needs to be available outside the binary, should encapsulate the thread safety part. Perhaps, PHP's own APIs can be also reused for some part, but in general there's probably no way around other than PHP-CPP to have own wrapper I/O classes. That's also the direction @atvise's thought goes to. Right now, VS build is just broken.

Just my 2 cents.

Thanks.