On 26 February 2014 11:35, Anatol Belski <[email protected]> wrote:
> Hi Chris,
>
> On Wed, February 26, 2014 12:09, Chris Wright wrote:
>> Hi Anatol
>>
>>
>> It's interesting that you should have a problem here, both Daniel and
>> I extensively "tested the tests" to ensure they would run and pass on
>> both windows and *nix - although I never tested on mac because I don't have
>> easy access to one (I'm not sure if Daniel did) but I assumed they would
>> work nicely there since they worked on all the *nix flavours I tested.
>> WHat OS are you running these on?
> It's win8 64 bit build of the PHP-5.6
>>
>> The diff you show doesn't look like it would fix the problem as it
>> seems to be waiting for eof on stdin in the worker, which shouldn't ever
>> happen until the test is completed in the main process (the handle isn't
>> closed until the main process code has finished executing, which if it
>> uses a wait() will never happen if the worker executes successfully.
> The issue I saw there is that while(1) has never exited because it's
> expected '---' to appear to break. The way i did it at least jumps out
> from the loop. For example stream_server_reneg_limit.phpt does something
> else, but also hangs forever similar way. Maybe the piece with '---'
> should be restored and '---' appended to all the tests, but actually it
> were better to not depend on that.
The reason it was designed like this is to allow two way communication
between the processes via their standard streams to remain open, so
that the wait/notify system works in both directions. As it stands
none of the tests use this, but I figured it was better to leave it
there for future use. It's somewhat concerning if --- never appears
in the stream and that loop runs forever, since it's explicitly added
as an end delimiter by spawnWorkerProcess()...
stream_server_reneg_client.phpt is actually something of a special
case. It's a dirty hack using an external openssl binary to trigger
renegotiations, as there's no way to trigger this directly in userland
PHP at the moment, and both Daniel and I had issues running this test
on Windows. As it turned out, the reason for this was actually a very
old and buggy openssl binary that ships with msysgit bins and so was
in %path% - when we installed a 1.0.1f binary from
http://slproweb.com/products/Win32OpenSSL.html
the test ran without
issues. I'm working to figure out exactly when the bug was fixed so we
can check for it in the SKIPIF.
Are you having issues with only certain tests or is it all of the modified ones?
>
>> I think it's likely that your machine is having difficulties launching
>> the background process for some reason, I suggest you inspect the $cmd path
>> generated in spawnWorkerProcess().
>>
> I've checked that by copying the server code explicitly and ran it in a
> separate process. The hang was still there (so not leaving the while(1)
> part), for instance with bug46127.phpt .
>
>> One thought that does occur is that it might be worth changing the
>> sprintf pattern to "%s" "%s" %s - if you had any spaces in the path to
>> the
>> PHP binary then the unquoted command would probably fail to
>> launch.
> There are no spaces in the paths, that's standard approach with
> c:\php-sdk\... I'll try this anyway.
OK that's not likely to be the issue here then, although we should
probably still make the change to cover this case.
> Have you tested also on 64 bit builds? Maybe it's specific to that. But
> the impression I have is that there's something in the PHP
> code/environment behaving different for whatever reason.
I've not run the tests against a 64 bit build, I did create a 64 bit
binary to verify that it worked but I ran the tests against a 32 bit
build.
>
>>
>> On 26 February 2014 10:39, Anatol Belski <[email protected]> wrote:
>>
>>> Hi Chris,
>>>
>>>
>>> On Wed, February 26, 2014 10:48, Chris Wright wrote:
>>>
>>>> On 22 February 2014 00:31, Chris Wright <[email protected]> wrote:
>>>>
>>>>
>>>>> Following on from this thread and Daniel's excellent work on TLS
>>>>> improvements, and liaising heavily with Daniel off-list, I have
>> created a
>>>>> PR [1] of some work I have done to get peer verification
>>>>> working with Windows native certificate store.
>>>>>
>>>>> This is by far and away the most preferable option as it gives "out
>>>>> of the box" support for peer verification by default on Windows, and
>>>>> does not require any additional certificate bundles or
>>>>> configuration. It
>> also
>>>>> allows us to take advantage of trust updates rolled out via MS
>>>>> update systems.
>>>>>
>>>>> The implementation is complete in that it supports all existing
>>>>> features, although it needs a little polishing and some edge cases
>>>>> covering before it can be merged. The only definite known issue at
>>>>> the time of writing is that the method for fetching the CN from the
>>>>> certificate incorrectly assumes that the returned data will always
>>>>> be UTF-8 encoded, a solution for this is planned and will be
>>>>> implemented in the next day or two.
>>>>>
>>>>> I am by no means an expert on the subject matter here in any
>>>>> respect, so I encourage ruthless code review.
>>>>>
>>>>> Note that there are no new features here, it is simply looking to
>>>>> fill in the gaps in the recent work by providing consistency on
>>>>> Windows.
>>>>>
>>>>>
>>>>> [1] https://github.com/php/php-src/pull/601
>>>>>
>>>>>
>>>>
>>>> This patch is now merged (thanks Daniel) and will be available in the
>>>> next alpha. If anyone finds anything that doesn't behave as
>>>> expected, please let me know.
>>>>
>>>
>>> the tests with the removed pcntl dependencies do fail for me. I made a
>>> small change here
>>>
>>>
>> http://git.php.net/?p=php-src.git;a=commitdiff;h=56cbe043810ab773605aa6a6
>> c a2eb362ea9a54e9
>>>
>>> but still there are some with the similar diff
>>>
>>> TEST 43/79
>>> [C:\php-sdk\php56\vc11\x64\php-src\ext\openssl\tests\bug65538_001.phpt]
>>> ========DIFF========
>>> 001+ Warning: file_get_contents(https://127.0.0.1:64321/): failed to
>>>
>> open
>>> stream: No connection could be made because the target machine actively
>>> refused it. 001- string(12) "Hello World!"
>>> 002+ in
>>>
>>>
>> C:\php-sdk\php56\vc11\x64\php-src\ext\openssl\tests\ServerClientTestCase.
>> i nc(93)
>>> : eval()'d code on line 8
>>> 003+ bool(false)
>>> ========DONE========
>>>
>>>
>>> Actually it's great to get rid of that pcntl dependency there, just we
>>> should bring it inline. Working on the further fixes.
>>>
>>> Regards
>>>
>>>
>>> Anatol
>>>
>>>
>>
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: http://www.php.net/unsub.php
>>
>>
> Regards
>
> Anatol
>