pytorch / pytorch/gloo

Conflicting comments and enforcement conditions in transport/tcp/buffer.cc

Open
#421 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
1.5k
Forks
368
Avg merge
19h 32m
Merged PRs (30d)
3

Description

Near transport/tcp/buffer.cc:91:

  if (pair_->isSync()) {
    // The send operation must flush all data to the underlying socket
    // and then call handleSendCompletion. Therefore, the number of
    // send completions must always be positive when calling waitSend.
    GLOO_ENFORCE_GE(1, sendCompletions_);

The comments says sendCompletions must be positive, while the enforcement condition says 1>=sendCompletions. It seems to me the latter is wrong.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read transport/tcp/buffer.cc around line 91 and compare the explanatory comment with the GLOO_ENFORCE_GE condition and sendCompletions_ usage. Check the surrounding synchronous send path and relevant TCP tests. Done means the enforcement condition and comment express the same requirement without changing unrelated behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
networking
Issue type
Bug
Difficulty
1/5
Estimated time
Under an hour
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.