Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(16400)

Issue 135740043: _ProactorBaseWritePipeTransport.write() pauses the protocol if needed

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 8 months ago by haypo_gmail
Modified:
9 years, 8 months ago
Reviewers:
yselivanov
Visibility:
Public.

Description

_ProactorBaseWritePipeTransport.write() pauses the protocol if needed

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -6 lines) Patch
M asyncio/proactor_events.py View 1 chunk +2 lines, -6 lines 0 comments Download

Messages

Total messages: 2
yselivanov
Is it possible at all to have a unittest, which will show that your change ...
9 years, 8 months ago (2014-08-25 15:44:49 UTC) #1
haypo_gmail
9 years, 8 months ago (2014-08-25 17:28:15 UTC) #2
On 2014/08/25 15:44:49, yselivanov wrote:
> Is it possible at all to have a unittest, which will show that your change is
> actually needed?

Hum, it's difficult to mock the write() method of each transport implementation,
because there are multiple transport implementations and each is very different.

Using real socket is not reliable, because all data may be written directly
depending on the speed of the computer, even if a local server is used and the
server doesn't read all written bytes.

The CPython test suite uses a limit of 16 MB + 1 byte to ensure that a send
blocks (Lib/test/support/__init__.py):
---
# A constant likely larger than the underlying OS socket buffer size, to make
# writes blocking.
# The socket buffer sizes can usually be tuned system-wide (e.g. through sysctl
# on Linux), or on a per-socket basis (SO_SNDBUF/SO_RCVBUF). See issue #18643
# for a discussion of this number).
SOCK_MAX_SIZE = 16 * 1024 * 1024 + 1
---

Here is an attempt to write such test:
http://codereview.appspot.com/126600043

The test pass on Linux, but fails on Windows because write() always completes
immediatly (even if the test writes 16 MB at once). I'm not sure that it's
possible to write a reliable and portable test this patch.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b