Error Display #75

Closed
olekukonko opened this Issue Mar 19, 2013 · 14 comments

4 participants

@olekukonko

If you run the code below on command line .. you get

    error_reporting(E_ALL);
    ini_set("display_errors", "On");
    class ThreadClass extends Thread {
        public function run() {
            error_reporting(E_ALL);
            new NonExstingClass();
        }
    }

    $g = new ThreadClass();
    $g->start();

PHP Fatal error: Class 'NonExstingClass' not found which is valid but if you run it via PHP 5.4.11 & Apache .. You get empty output ..... no error is displayed

@krakjoe
Owner

The output stream of the thread in apache is different to that in CLI, it is not directly handled by PHP, there is nothing we can do to affect it.

@krakjoe krakjoe closed this Mar 19, 2013
@olekukonko

Is there any work around ? They can be somany things that can go wrong , Memory , Timeout etc that can generate Fatal error

I tried setting Custom error handler using set_error_handler but it did not not work yet register_shutdown_function working perfectly


    error_reporting(E_ALL);
    ini_set("display_errors", "On");

    function myErrorHandler($errno, $errstr, $errfile, $errline) {
        var_dump($errno, $errstr, $errfile, $errline);
    }

    function shutdown() {
        var_dump("Shutting Down");
    }

    class ThreadClass extends Thread {
        public function run() {
            error_reporting(E_ALL);
            set_error_handler("myErrorHandler");
            register_shutdown_function("shutdown");
            notexistfunc();

        }
    }

    $g = new ThreadClass();
    $g->start();

Any suggestions would be appreciated

@krakjoe
Owner

There is no workaround to get you access to the standard output of apache, and you should not try to find one.

Instead, adopt error logging to a file so you can see the errors from threads during execution. An error logging object should open the same file in write mode in every thread, stream as a static resource ( ensuring it is recreated for every context ), it should acquire a mutex before it writes to the file so output is readable, using a protected method may work, but being that the error handler may be called in a non uniform way from other extensions a mutex would be more reliable.

Are you using OpCache/Zend Optimizer+ ? I know from recent posts on mailing lists that it does not call user set error handlers during execution, so unload that ( it doesn't really do anything in the context of threads because pthreads copies everything it is caching ).

Hopefully that'll get you somewhere ?

@olekukonko

Hi,

The error was properly logged .....Am not using OpCache/Zend Optimizer. What if you are not to capture the actual error but show what the class was terminated prematurely ?

Example



    class ThreadClass extends Thread {
        private $isCompleted = false;

        public function run() {
            sleep(1); // doing somthing
            notexistfunc(); // Fatal error
            $this->isCompleted = true;
        }

        public function isCompleted() {
            return $this->isCompleted;
        }
    }

    $g = new ThreadClass();
    $g->start();

    if ($g->join()) {
        if (! $g->isCompleted()) {
            throw new ErrorException("Check the crazy error log");
        }
    }

This way its optional to check if Thread::isCompleted() rather than assume. Am just trying to see a way we can make this manageable .

@krakjoe
Owner

Good idea :)

so now, we detect fatal errors and uncaught exceptions on threads and stackables during execution, I give you ... isTerminated ... please test ...

At some time in the future, we might save uncaught exceptions, though I am reluctant at this stage to change the definition of a pthreads object, or make it any larger... statefulness should be enough for now ... there is no way we can catch the content of fatal errors I don't think, at least certainly not in a compatible and maintainable way.

@olekukonko

Well done .... 👍

isTerminated is a good method to add. I would continue my testing and get back to you .... Where can i get the updated binaries

It would be nice if this can make it to the PHP core distribution .. have been tell a lot of people about it in http://stackoverflow.com/

Nice One ....

@chibisuke

display_errors is causing lots of trouble in my oppinion and is just not worth using it at all.
On one hand it can trash your whole HTML layout, on the other hand it gives away information on your system that you might not want other ppl to have (IP of your databases for example).

I'm using display_errors=no, log_errors=yes, error_log=/var/log/nginx/php-error.log in my php.ini and it works fine at least with php-fpm. Don't know about apache however.

@krakjoe
Owner

@olekukonko updated binaries to come yet, once we test on unix/mac and our various distros and nobody reports any bugs I'll send out a release. 0.43 should be coming up in the next few days ...

http://pthreads.org/snapshots/5.4/0d5df071231478ff3b1982eb2b0d96cc24e39d9a.zip ... snapshot for you to test

@shoghicp if you could get your users to test these binaries too before the release in a few days that'd be great ... these were built with windows 7, vs2008, the pthread-w32 dll included was built on the same system at the same time, so there should be no incompatibilities.

about being part of the core ... I am not sure that is a good idea, this is complex in comparison to normal PHP, if you find it easy then it's great for you, but we must acknowledge to most it is not, and PHP must remain easy to maintain it's position in the world of web programming, so I'm quite happy for it not to be in the core ... I would be equally happy to advance its status one day such that it could be included and take advantage of internal API's I know exist and had to drop in order for everyone to be able to build shared libraries, either way I'm not fussed and I won't be moving in any direction on my own, if people get behind it, I'll get behind it ...

@chibisuke you can ini_set in threaded contexts, or use error_reporting in threads can't you ?? The problem is if we pick and choose INI settings we have to inspect every setting to the same degree, which for systems for hundreds of ini directives could be detrimental ...

@shoghicp

Going to test it, also with PHP 5.4.13

@olekukonko

@krakjoe thanks for the snapshot .. would run some test and get back you .....

@olekukonko

Does this functionality only benefit Thread or Worker and Stackable also ? looking at your example at https://github.com/krakjoe/pthreads/blob/master/examples/SQLWorker.php Fatal Error can occur also

@krakjoe
Owner

Thread and Stackable.

You don't interact with the state of a worker to the same degree as Threads and Stackables since some of it's functionality as a thread is overridden to turn it into a Worker. Additionally, in a worker/stackable scenario, the unit of execution is supposed to be considered the stackable, the worker is just the environment, whereas in a basic Thread scenario the Thread itself is the unit of execution.

Where's the possible error in SQLWorker ?

I must do something about the tutorials/examples situation, I am on holiday from work right now and have all the time in the world, soon enough I have will have no time for very long periods ... it would be nice to get some volunteers to maintain some tutorials/docs, I just don't have the time for this sort of thing ordinarily which is why they are in the state they are in ...

@olekukonko

Thread::isTerminated works perfectly fine ... is it possible to add make sure join was called before Thread::isTerminated

Example

class ThreadClass extends Thread {
    public function run() {
        sleep(3);
        NonExistingFunction(); // Fatal error
    }
}
$s = new ThreadClass();
$s->start();
$s->join();
var_dump($s->isTerminated());
  • Expected :True
  • Actual : True
    $s = new ThreadClass();
    $s->start();
    //$s->join();
    var_dump($s->isTerminated());
  • Expected : true
  • Actual : false

Suggestion

if(Thread is not Join)
    {
        .... call join or wait for join then 
    }
then return isTerminated 
@krakjoe
Owner

That belongs in userland. isTerminated is for detection of state at the time of the call, it should not "do" anything.

On a side note, steer clear of sleep, your thread is not in a responsive state while sleeping. Instead use wait() provided such that if your application needs to, it can notify() the object/context to wake and continue on demand ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment