Paths

Table of Contentst

Differential D7772

Add a maximum read buffer size to ExecFuture
ClosedPublic
Actions

Authored by epriestley on Dec 16 2013, 6:16 PM.

Details

Reviewers

btrahan

Maniphest Tasks

T4241: Cloning repository keeps entire repository in memory as it's being streamed over SSH

Commits

rPHU898cdef4c957: Add a maximum read buffer size to ExecFuture

Summary

Ref T4241. Currently, ExecFuture may read an arbitrarily large amount of data into memory during execution. In T4241, this arises specifically with git-upload-pack, which emits about as much data as the size of the repository. Currently, the subprocess emits this data far faster than we can send it over the network, so we end up with a huge in-process buffer containing the entire repository.

Instead, we want to throttle git-upload-pack until the network can catch up. This is the first step in letting that happen.

Test Plan

Added a unit test.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

Use slightly clearer variable names.

btrahan accepted this revision.Dec 16 2013, 6:35 PM

btrahan added inline comments.

src/future/exec/ExecFuture.php
534–541	just wanted to double check if any other callsites - particular in children classes - need to be updated as I think not specifying length will trigger the early return.

epriestley added inline comments.Dec 16 2013, 6:39 PM

src/future/exec/ExecFuture.php
168	(This is the first in a series of several complicated diffs, and I just wanted to limit the amount of extra stuff happening for now.)
534–541	Ah, yeah -- a lot of `protected` in this class should really be `private`, it's just super old -- there are no other callsites and the class is `final`. I'll send a followup.

Closed by commit rPHU898cdef4c957 (authored by @epriestley).

could you help me to solve this problem as soon as possible, thank you very much

src/future/exec/ExecFuture.php
542	fread() will be blocked , when the content of stdout is too much

my PHP version is php-5.4.17

window xp environment

hey @xuyulong - are you trying to run Phabricator on windows? That is currently (and likely to remain) not supported. Otherwise, what Arcanist or other workflows are you encountering to hit this error?

yes，i am run phabricator on windows,but, fread() will not block when my cpp files smaller than 3k and the fread() will be blocked when the cpp file more than 3.26 K

See T3272. Windows doesn't have async pipes, so it isn't clear how to deal with this. T3272 has a possible solution (redirect the pipes through files, essentially).

thank you very much!!!

Revision Contents
Changeset List

Path

Size

src/

future/

exec/

ExecFuture.php

66 lines

__tests__/

ExecFutureTestCase.php

40 lines

Diff 17588

View Options

src/future/exec/ExecFuture.php

View Options

src/future/exec/tests/ExecFutureTestCase.php

Add a maximum read buffer size to ExecFutureClosedPublicActions