PHP 7.2 Release

[tpunt]

Ping @sirsnyder

I believe Joe is assuming we will make the next release (judging by his comment here).

Before we make the next release, what needs to be done on Sockets to stabilise it a bit more? Should we either revert the recent changes and place them into their own branch, or do they just require a bit more work? I think this is the primary blocking issue for a stable release.

Hopefully, if we can sort out the sockets stuff over the coming days, then we can make a new release over the weekend.

[tpunt]

Also ping @dktapps, as someone who has been working on the sockets stuff recently.

[sirsnyder]

Good question. We should keep them, cause I think Sockets are a high anticipated feature.

In case of the primary blocking issue, I could today and tomorrow compare all functions of our current socket implementation with the php socket behavior and adapt it. Of course if possible with your support and/or @dktapps. I would not add #744 yet, but #804 and also I would be very much in favor to add #778. Volatile objects are error prone in complex object structures. PR #778 fixes the well-known Exception "pthreads detected an attempt to connect to an object which has already been destroyed".

[tpunt]

Ok, if you could work on the sockets stuff today/tomorrow, then that would be great. I should be able to find some time to review #778, but I won't be able to help with the sockets stuff as well (hopefully @dktapps can help you with that instead).

[dktapps]

I've been trying to get my guinea pigs to test #778 recently due to a wide range of bug reports I keep getting relating to multidimensional arrays segfault issues and such. I'll see what I can do.

I can assist with socket-related stuff if needed. The primary showstopper related specifically to sockets at the moment is (in my opinion) issues with throwing exceptions in non-blocking mode (such as #809 ). I don't think there are any stability issues wrt Sockets right now apart from odd bugs - the rest is just missing features.

[dktapps]

There are also a bunch of cases of misbehaving Socket tests that may need investigation.

[sirsnyder]

@ which socket tests do you mean? Feel free to correct them ;-)

[dktapps]

@sirsnyder I keep seeing the socket-bind test failing for no apparent reason. It works fine when testing locally so maybe some Travis/AppVeyor resources issue, I don't know.

[sirsnyder]

@dktapps very strange the behavior of this one test on x64 arch.
@tpunt the sockets stuff has been merged into master.

[tpunt]

@sirsnyder Thanks. Assuming nothing else comes up, I'll make a new release over this weekend :)

[dktapps]

@sirsnyder I wouldn't swear to this but it may be some issue with AppVeyor, because I've seen it randomly fail on x86 and x64 both. I can't reproduce it when running tests locally at all, on either arch.

[dktapps]

FWIW I'm not sure what versioning system pthreads follows, but I think sockets (as addition of features) warrant a minor version bump to 3.2.0.

(also, 3.1.7 is ancient now, running for 2 years)

[tpunt]

@dktapps Yep, that's exactly my thinking too. It will be a 3.2.0 release.

[sirsnyder]

@dktapps will have a look next days or week, it's low prio for me.
@tpunt nice to read :) At least it should be a 3.2.0 release. Maybe even a 4.0.0 release, because there is a BC break with 465d3bc and 1fcafd7

[dktapps]

wrt. socket-bind.phpt, there is a random element in the test which I didn't initially notice, which might explain the inconsistency of the test failures. I'll take a look over the weekend and see if I can get it to reproduce locally.

[dktapps]

Something that concerns me (not completely related to this issue, but bear with me) is the global Socket namespace. If ext/sockets happens to be upgraded to drop resources in the future (like ext/gmp did in PHP 5.6) then we will have a conflict on our hands if ext/sockets also uses the global Socket classname.

I'm not really sure what can be done about this in consistency's name (maybe ThreadedSocket or a pthreads namespace), but if it's going to be changed it might want to be done before the API is finalized in a release.

With this in mind, it might also not be a bad idea to have some kind of --disable-pthreads-sockets compile-time option.

[dktapps]

Hi guys. Any movement on this?

[tpunt]

@dktapps I've been rather busy over the past couple of weeks, but now I have some more free time again. I'll be working on pthreads some more over the coming days.

[dktapps]

@tpunt that's good to hear!

I've been re-reading sockets with intent to use them in my project in the future and I'm realizing that there's been a lot of nasty legacy things introduced into there from php-src - something|false returns for example (can't be typehinted against, evaluate to zero, etc), silent false returns on bad arguments, EINVAL being silenced (#863 ), and a bunch of other things.

I think given that pthreads prefers an exception-based approach overall, more strictness should be applied wrt. invalid arguments (throwing exceptions), and additionally avoid |bool returns - in most cases the C APIs for sockets will return negative values or whatever that could be returned to the user directly, and in other cases it would be better to use |null (for typehints' sake).

If there's one thing I really don't like about ext/sockets, it's the way it loves to return false for all kinds of things, so I think that's better avoided here since we have better things to play with (exceptions and such).

[dktapps]

I'm also noticing just now that RuntimeException has been used in a lot of places where InvalidArgumentException would be more suitable. Rather inconsistent to say the least.

[snowleopard24]

@tpunt , @sirsnyder reading the threads from Feb it seems we will have a 3.2.0 version. How close are you guys to the release of 3.2.0 and subsequently pushing for snapshot PECL build of pthread for windows?

[tpunt]

@snowleopard24 It's difficult to say. I'm a bit pushed for time currently, so I haven't had the chance to review any PRs. I hope to find some more time soon for this, though.

[sirsnyder]

Time flies by ... and we need a release. Please let us merge #882 (and if possible #831) within the next days to be php 7.3 compliant and release pthreads 3.2. The Socket class is still experimental, but that's the way it is. The last release was almost 2 years ago, a change log would be massive and takes
quite some time, so I would drop it exceptionally for this release.

@tpunt I guess it's easy but I have no idea what is necessary to push a release, especially in terms of pecl. If you tell me the steps, I will trigger releases in future regularly.

After 3.2 I favor december or january for a 4.0 release containing at least the following points

• Stable Socket class
• Feature WIP: New Concurrent class #906, either as separate class or integrated into Threadable and Volatile. Depends on the benchmark.
• Bugfix Add test for crash when modifying static array after thread start #861, will be very likely a BC break!

BTW, I think the adjustments for php 7.4 are very extensive and need a lot of time.

[tpunt]

@sirsnyder This sounds good to me. Regarding the PECL release, I don't usually bother with this for my extensions, so I'm not sure how it is done exactly. I know who we can ping for Windows builds on PECL, though. So I'd say just focus on merging those couple of pull requests, and then make a release on this repo.

I will try to review the PHP 7.3 updates sometime over the next week to ensure that everything is fine. I don't really have many comments on #831 - it is probably best for Joe to review this.

[snowleopard24]

Looking at PECL releases, the last release was v3.1.6 on 5/25/2018 7:31 AM. And, if I'm not wrong pthreads v3.1.6 was php 7.2 compliant.
@tpunt , @sirsnyder Are we saying that the next pthreads PECL release would be php 7.3 compliant?
I have to upgrade my server and I was wondering if I shall go ahead with pthreads v3.1.6 /w php 7.2 or wait for next release of pthreads /w php 7.3 if that's not too far.

[sirsnyder]

Release v3.2.0 is live. You will find *.dlls in the release. Alternatively is an official pecl snapshot available.