Command hangs forever after update from 1.2.2 to 1.2.3

[schmunk42]

Hanging test: https://github.com/dmstr/phd5-app/blob/master/tests/codeception/cli/DbDumpCept.php

Output:

Cli Tests (5) ------------------------------------------------------------------
✔ 1-InitCept:  (4.90s)
✔ AuditCept:  (0.10s)
make: *** [run-tests] Error 137
stdin: is not a tty
ERROR: Build failed: execution took longer than 3600 seconds

It's a rather simple DB-dump which takes <1s in the test setup.

Works fine after reverting to 1.2.2

[schmunk42]

[Update]

Source of the tested command: https://github.com/dmstr/yii2-db/blob/master/console/controllers/MysqlController.php#L232

CC: @Quexer69 @handcode

[mikehaertl]

@schmunk42 Ok, so the change suggested in #20 now introduces other problems. It seems, whichever way we do it, there's always a situation where the command can hang. And I want to avoid introducing more esotheric settings to make this configurable.

Can we find a silver bullet that always works?

CC: @leonardopcastro

[mikehaertl]

@schmunk42 @leonardopcastro The first thing we should do is write tests that reproduce the issues described here and in #20. Could you maybe help here? I personally never had an issue with hanging commands like this.

[leopcastro]

I will try to isolate the problem in my configuration to check if it's OS related, PHP related, or if some other variable is influencing it.

[leopcastro]

I managed to find out what is the situation that triggers the hanging error described in #20. It happens when using mikehaertl\wkhtmlto\Pdf class with a long html string. If you use the Pdf class with a file name, it is working as intended.

I attached two sample files so you can reproduce the issue too. I changed the extension of them because Github would not allow me to attach the originals. At the end of the html file, you are going to find these numbers "12345678". If you erase one of them, the error message of the wkhtmltopdf will show up and the php will not hang.

long_test_html.txt
wkhtmltopdfTest_php.txt

[mikehaertl]

@leonardopcastro Hmm, ok thanks. To be honest I was hoping for a simpler test. We need to write an automated test case and pulling in our wkhtmltopdf library for this seems like overkill. It's also harder to really find the root of the problem. phpwkthmltopdf does a couple of things under the hood if you generate a PDF from a string.

It would be great if we could reproduce the issue by using only standard shell commands like cat, grep, echo, sleep etc. to reproduce the issue. Any idea? @schmunk42 maybe?

[schmunk42]

I'd try to reproduce the case mentioned, the file is around 61kB, maybe there's some sort of 64kB buffer somewhere causing this.

@leonardopcastro Some full code including the initial php-shellcommand object would be helpful.

[schmunk42]

And I want to avoid introducing more esotheric settings to make this configurable.

Hmm, the current situation is somehow not ideal, since we have to pin the version of shellcommand to 1.2.2.

Couldn't we make this configurable after all? Like $streamOrder = ['stdOut','stdErr']

[mikehaertl]

I really want to avoid that. A setting like this implies, that the developers knows about the ugly internals of this class. We should not annoy our users with this.

So for now I think you have to pin the version to 1.2.2. Any progress on a test case? I can also have a look but it can take a while.

[schmunk42]

So for now I think you have to pin the version to 1.2.2.

I mean there are no other issues reported on either side, but concerning semantic versioning you should revert this in 1.2.4 and create a 1.3.0-beta1 until this is finally resolved.

[mikehaertl]

@schmunk42 I've released 1.2.4 which basically reverts the changes introduced in #20. Until we find a solution without breaking BC this should be the best option. I don't think it's neccessary to create 1.3.0 as our final goal should be, not to have any BC break at all.

@leonardopcastro So it's now you who have to pin your version to 1.2.3 ;). Let's continue discussion in #20. Still interested in a simple test case for both problems.