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

Buffering writes when writing wouldn't block #15

Closed
mbonneau opened this issue Oct 15, 2014 · 4 comments
Closed

Buffering writes when writing wouldn't block #15

mbonneau opened this issue Oct 15, 2014 · 4 comments

Comments

@mbonneau
Copy link

I was working on Thruway and created a client that did non-trivial blocking I/O in response to an invocation message. While it was doing this work, more invocation messages came in and buffered on the read side of the stream. The first task completed and "wrote" to the stream. Because there is no attempt to actually write at that moment (even though it would not have blocked), the next command is read and the process begins work on that - leaving data in the write buffer until that is done (or possibly until more messages and responses are completed, depending on read-side message processing).

I was able to fix the problem by calling $this->handleWrite() immediately after $this->loop->addWriteStream($this->stream, [$this, 'handleWrite']); in Buffer.php.

I do suppose this could cause issues with handleWrite because it is possible that the write would block and get an EWOULDBLOCK error (I think), which it currently does not appear to handle.

This change also causes the unit testing to fail.

1) React\Tests\Stream\BufferTest::testWriteReturnsFalseWhenBufferIsFull
Failed asserting that true is false.

/Applications/MAMP/htdocs/thruway_website/vendor/react/stream/tests/BufferTest.php:54

2) React\Tests\Stream\BufferTest::testWriteDetectsWhenOtherSideIsClosed
React\Tests\Stream\CallableStub::__invoke(RuntimeException Object (...), React\Stream\Buffer Object (...)) was not expected to be called more than once.

I have tried this with both StreamSelectLoop and LibEventLoop and get the same behavior. (This buffering buffers and the fix above fixes it.)

@steverhoades
Copy link
Contributor

I am having a hard time following what the issue is that your describing. I'll summarize what I think I understand.

  1. Within the server you are purposely "blocking" when an invocation request is received
  2. Your write doesn't execute immediately as you would like, instead more data is read

Please correct me if i've misunderstood this.

So some important information regarding the event-loop in react. It runs in a single thread of context, meaning that it is not actually running anything in parallel. The event-loop listens for read/write events on resources (sockets in your case) and will then execute the callbacks serially.

When you request a write on a resource, react will first add that resource to the event-loop to ensure the resource is ready to be written to. Refer to the paragraph above to understand how the callback to handleWrite will be executed.

I would strongly recommend that if you must "BLOCK" your server, you hand the blocking work off to another process - you should never purposefully block the event-loop. Take a look at react/child-process, fast-cgi to php-fpm or even pthreads.

@clue
Copy link
Member

clue commented Jun 7, 2015

blocking I/O

I'm not sure it's likely we'll see support for blocking functions any time soon, given that even the FAQs warn against this (see also discussion in #16).

I would strongly recommend that if you must "BLOCK" your server, you hand the blocking work off to another process - you should never purposefully block the event-loop. Take a look at react/child-process, fast-cgi to php-fpm or even pthreads.

👍 on this as an alternative

Any thoughts?

@mbonneau
Copy link
Author

To clarify: I was suggesting that write operations be attempted immediately without returning to the event-loop. If there are operations that take processing time but return data throughout the processing, it is buffered right now, waiting until control goes back to the loop before writing.

This issue is not particularly important to me, so I am going to close it.

@clue
Copy link
Member

clue commented Aug 21, 2016

Thanks for your suggestion @mbonneau, it kind of looks like we talked past each other back then :-) I've just filed #54 which implements have you seem to have suggested here first 👍

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

No branches or pull requests

3 participants