Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

make collect() respect stream pause/resume state #53

Closed
wants to merge 2 commits into from
Closed

make collect() respect stream pause/resume state #53

wants to merge 2 commits into from

Conversation

rvagg
Copy link
Contributor

@rvagg rvagg commented Jun 17, 2016

  • Reverts a55ae72 / @evanlucas / as this only masks part of the problem and ends up with an incorrect list of entries anyway.
  • Adds a line in collect() that makes it properly respect state so the ignore list gets to be properly built before fstream starts reading entries.

Explained in commit message:

Without watching if a stream is already paused before calling pause(),
collect() can conflict with other users of the stream who may be
managing state, resulting in a premature collect().
This manifests in the fstream-npm and node-tar combination where
fstream-npm manages state in readBundledLinks() at the same time
collect() is processing the stream. collect() ends up running before
fstream-npm has properly set up the correct list of entries according
to the ignore rules. When collect() gets to run before this is complete,
fstream starts performing _read() operations on a non-filtered list,
keeping track of entry index, then after it starts, fstream-npm inserts
a new list of entries and the entry index is incorrect and can skip
files.

rvagg added 2 commits June 17, 2016 22:11
This reverts commit a55ae72.

Unnecessary when collect() handles pause/resume state properly, this
compounds the issue and adds unnecessary entries to the list.
Addressing npm/npm#5082

Without watching if a stream is already paused before calling pause(),
collect() can conflict with other users of the stream who may be
managing state, resulting in a premature collect().
This manifests in the fstream-npm and node-tar combination where
fstream-npm manages state in readBundledLinks() at the same time
collect() is processing the stream. collect() ends up running before
fstream-npm has properly set up the _correct_ list of entries according
to the ignore rules. When collect() gets to run before this is complete,
fstream starts performing _read() operations on a non-filtered list,
keeping track of entry index, then after it starts, fstream-npm inserts
a new list of entries and the entry index is incorrect and can skip
files.
@SimenB
Copy link

SimenB commented Jun 17, 2016

1 line of code. GG good sir!

othiym23 pushed a commit that referenced this pull request Jun 17, 2016
Unnecessary when collect() handles pause/resume state properly, this
compounds the issue and adds unnecessary entries to the list.

Credit: @rvagg
Reviewed-By: @othiym23
PR-URL: #53
othiym23 pushed a commit that referenced this pull request Jun 17, 2016
Without watching if a stream is already paused before calling pause(),
collect() can conflict with other users of the stream who may be
managing state, resulting in a premature collect().  This manifests in
the fstream-npm and node-tar combination where fstream-npm manages state
in readBundledLinks() at the same time collect() is processing the
stream. collect() ends up running before fstream-npm has properly set up
the _correct_ list of entries according to the ignore rules. When
collect() gets to run before this is complete, fstream starts performing
_read() operations on a non-filtered list, keeping track of entry index,
then after it starts, fstream-npm inserts a new list of entries and the
entry index is incorrect and can skip files.

Fixes: npm/npm#5082
Credit: @rvagg
Reviewed-By: @othiym23
PR-URL: #53
@othiym23
Copy link
Contributor

Landed as a67b90b and 24c2ad5, released as [email protected]. Thanks a ton, Rod!

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

Successfully merging this pull request may close these issues.

3 participants