This is an archive of the discontinued Mercurial Phabricator instance.

bundle2: move processpart stream maintenance into part iterator
ClosedPublic

Authored by durham on Sep 13 2017, 8:38 PM.

Details

Summary

The processpart function also did some stream maintenance, so let's move it to
the part iterator as well, as part of moving all part iteration logic into the
class.

There is one place processpart is called outside of the normal loop, so we
manually handle the seek there.

The now-empty try/finally will be removed in a later patch, for ease of review.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

durham created this revision.Sep 13 2017, 8:38 PM
indygreg requested changes to this revision.Sep 14 2017, 12:04 AM
indygreg added a subscriber: indygreg.
indygreg added inline comments.
mercurial/bundle2.py
1149–1152

I think there is a change in behavior here.

Before, _processpart() would catch SystemExit and KeyboardInterrupt and not attempt part.seek(). After this change, it always does the part.seek() courtesy of being in a finally block.

The problem with this is that we may attempt I/O after process termination is requested. This can lead to bad things, as the commit message for 9a9629b9416c details.

We mustn't attempt possibly blocking I/O after process termination is requested.

This revision now requires changes to proceed.Sep 14 2017, 12:04 AM
durham updated this revision to Diff 1818.Sep 14 2017, 1:21 PM
durham marked an inline comment as done.Sep 15 2017, 12:00 PM
This revision was automatically updated to reflect the committed changes.