If tick is called in futureTickQueue we got exception #60

Activity

1. nick4fake

   added a commit that references this issue on Nov 16, 2016

   Fixes reactphp#60: check for queue length in FutureTickQueue::tick()

   This issue will close once commit e5f4054 is merged into the 'master' branch.

   e5f4054
2. clue commented on Nov 16, 2016

   

   clue

   on Nov 16, 2016

   Member

   More actions

   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.
3. clue

   added

   question

   on Nov 17, 2016
4. nick4fake commented on Nov 18, 2016

   

   nick4fake

   on Nov 18, 2016

   Author

   More actions

   @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?
5. nick4fake commented on Nov 18, 2016

   

   nick4fake

   on Nov 18, 2016

   Author

   More actions

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

   What if we make a call inside a loop?
6. clue commented on Nov 18, 2016

   

   clue

   on Nov 18, 2016

   Member

   More actions

   Let's ping @WyriHaximus and let's see what he has to add 👍
7. WyriHaximus commented on Nov 18, 2016

   

   WyriHaximus

   on Nov 18, 2016

   Member

   More actions

   @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 😞 .
8. clue commented on Nov 19, 2016

   

   clue

   on Nov 19, 2016

   Member

   More actions

   @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?)?
9. clue commented on Feb 8, 2017

   

   clue

   on Feb 8, 2017

   Member

   More actions

   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 👍
10. clue

    closed this as completedon Feb 8, 2017