Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If tick is called in futureTickQueue we got exception #60

Closed
nick4fake opened this issue Nov 16, 2016 · 7 comments
Closed

If tick is called in futureTickQueue we got exception #60

nick4fake opened this issue Nov 16, 2016 · 7 comments
Labels

Comments

@nick4fake
Copy link

Hi,

On our project we call futureTick() in case in stream handler we need to wait for new data to populate. It has internal call to tick() that eventually clears current futureTick queue. In FutureTickQueue in tick() we simply call dequeue() exact amount of times equal to queue length at the tick() beginning. The problem is that we don't check if queue is already empty.

nick4fake pushed a commit to Werkint/reactphp-event-loop that referenced this issue Nov 16, 2016
@clue
Copy link
Member

clue commented Nov 16, 2016

I'm not sure I follow what you're doing, can you provide a gist to reproduce this?

In case you're calling tick() will the loop is running, this is probably where the problem comes from. This should not be done, ever.

@clue clue added the question label Nov 17, 2016
@nick4fake
Copy link
Author

@clue Please check this library:
https://github.com/WyriHaximus/reactphp-psr7-stream-converter/blob/master/src/ReactToPSR7Stream.php

Here tick() is called if on read there is no data. If I understand correctly, this is for loop not to block.

I use futureTick to queue read/write pipeline, eg:

public function tick()
{
    while (true) {
        $data = $this->input->read(self::BUFFER);
        if ('' === $data) {
            break;
        }

        // TODO: limit write rate
        $this->output->write($data);
    }

    if (!$this->input->eof()) {
        $this->queueTick();
        return;
    }
    $this->input->close();
    $this->output->close();
    call_user_func($this->result);
}

protected function queueTick()
{
    $this->loop->futureTick([$this, 'tick']);
}

Is this a correct way to implement PSR7->PSR7 pipeline?

@nick4fake
Copy link
Author

Another example:
https://github.com/WyriHaximus/ReactGuzzle/blob/master/src/HttpClientAdapter.php

What if we make a call inside a loop?

@clue
Copy link
Member

clue commented Nov 18, 2016

Let's ping @WyriHaximus and let's see what he has to add 👍

@WyriHaximus
Copy link
Member

@nick4fake it's a horrible hack that I would not recomend 😞 , heck I strongly discourage of doing it when there are alternatives. It 'works' but only in most circumstances. That one in particular was made to provide a fully working PSR-7 stream. @clue made a much better way of doing that here. Another place I use it is here, but there I'm doing it because Guzzle uses some funky queue that I have to keep running in order for the adapter to work 😞 .

@clue
Copy link
Member

clue commented Nov 19, 2016

@WyriHaximus thanks for giving some insight! What do you suggest we should do here? Is this something worth fixing or should we look into avoiding this altogether (is this even reasonable?)?

@clue
Copy link
Member

clue commented Feb 8, 2017

I believe this has been superseded now that #72 is in and the tick() method has been removed 👍

I'm closing this for now, please feel free to come back with more details if this problem persists and we can reopen this 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants