It appears this is to do with the array serialization happening on the wrong thread.

pthreads serializes and unserializes statics on the child thread. The problem with this is that serialization can cause objects to be modified (for example reallocating the properties table when it doesn't exist), which then cause memory errors later on when they try to be destroyed on the main thread.

The solution to this appears to be to do all static serialization on the main thread before thread start, and then unserialize on the child.

A similar issue is in effect with the exception-handler-options test failure - see pmmp/ext-pmmpthread@0c28adb for a solution.

Obviously a similar solution for statics would be rather more complex, and I really don't want to go down this avenue, but I see no other solution to the problem.

I've spent some days with research, debugging and tests and come to same conclusion. My approach was, to initially clean copy the objects to be serialized but even that was error prune. As an example, zend_array_dup allocates a new Hashtable though, but the containing zvals are shared (also their zend_refcounted object) and their refcounter incremented. If you just replace the zval with a new one, it leaks at shutdown, cause of the wrong refcount. A call of zval_ptr_dtor adds the zval of Thread 1 to the GC (as root object) of Thread 2 and that is one reason (of several possible) for the shutdown segfault.

At the moment I assume that I will take over your approach.

My solution is a nasty one. I'd love to see a better one if you have any ideas.

I'm dicing with the thought of an intermediary pthreads_store (like the property store, but inaccessible to user code) which things would be fed through from the main thread. That's a nasty solution also, but less nasty than others I had in mind.

I also experimented with switching TSRM contexts, but didn't get anywhere with that (perhaps because of TSRM static cache, but I barely have an idea what I'm doing there, so it's dangerous territory).

I think your exception handler solution is basically ok. But the situation with static properties is difficult. I thought about a separate pthreads_store too. Problem here is however, the zend vm doesn't provide any handler to read/write of static properties. At least I can't find any in code or docs.

Read/write is thread local, so that isn't an issue. There just needs to be an intermediary to copying statics when classes are copied to the child thread (on thread start and on worker task submit).

Jep, these are thread local. But what bothers me, would be the gap between serialization in pthreads_prepare_parent and unserialization later on in the new thread. In this time, there can happen a lot. Static properties may change, classes were added and also small potential for race conditions. With a proper vm handler, pthreads could track static properties continously. I'm still thinking about that.

In all honesty, the selective inheritance of statics (inherit primitives and arrays) is confusing and a royal pain in the ass. Real thread-locals do not behave this way (for example in C++11), so I do not see why they should in pthreads. Instead I think copying should be confined to only default static members, and not assigned values. This would kill several birds with one stone, at the expense of a BC break.

Additionally this may also be a problem with local static variables, but I haven't tested this.

I'm completely with you. I've a branch(sirsnyder@ec8fad8) in my fork, to stay compatible with the latest official release 3.1.6. The BC break is another point. We had so many changes and enhancements in the last two years, maybe we should go up to pthreads 4.

I'll change the copying within the next days as you suggested. At the moment I'm still busy with #873 ;-) Local static variables should not be affected. As far as I know, they are copied in pthreads_copy_user_function

From what I could see, anything where the serialization happened in the wrong context could potentially be affected. Constants would also be, but there are no ways to define complex constants which would reproduce this bug. Local static variables also - they exhibit the same behaviour as class statics wrt. copying, so I would expect they would be impacted by this bug too, but I'd need to test that.

A first draft 00779ab. Works very well with your attached statics-modify-copied-object-array test. Three tests are failing because of the BC break.

One thing I don't understand on further investigation. pthreads is already only copying the default_static_members_table, which is apparently not default static members (as far as I can tell). Bug in php-src or no?

1238300

In statics branch is some work on fixing this problem ... you may develop the store further using the copy routines that I've nicked from krakjoe/sandbox ... one test is failing, I dunno why ... this test passes but I didn't commit it ...

I may come back to this one weekend, or you may holla to ask questions, but I think you'll probably get it before me ;)