Skip to content

go: avoid possible overflow of the Write window#619

Merged
djs55 merged 1 commit intomoby:masterfrom
djs55:window-write
Feb 21, 2023
Merged

go: avoid possible overflow of the Write window#619
djs55 merged 1 commit intomoby:masterfrom
djs55:window-write

Conversation

@djs55
Copy link
Copy Markdown
Collaborator

@djs55 djs55 commented Feb 17, 2023

The Write window is used to keep track of how much buffer space is free in the remote to avoid one connection blocking the rest. Previously we checked the window and decided how much to write, then dropped the metadata mutex before performing the write. In theory another Write call on the same connection could see that buffer size is free, send too much and block the connection.

Therefore Write should take ownership of the space by bumping the current window before dropping the lock.

The Write window is used to keep track of how much buffer space is
free in the remote to avoid one connection blocking the rest.
Previously we checked the window and decided how much to write, then
dropped the metadata mutex before performing the write. In theory
another Write call on the same connection could see that buffer
size is free, send too much and block the connection.

Therefore Write should take ownership of the space by bumping the
`current` window before dropping the lock.

Signed-off-by: David Scott <dave@recoil.org>
@djs55 djs55 changed the title go: avoid possible read/modify/write of the Write window go: avoid possible overflow of the Write window Feb 17, 2023
Copy link
Copy Markdown
Collaborator

@fredericdalleau fredericdalleau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@djs55 djs55 merged commit 4786234 into moby:master Feb 21, 2023
@djs55 djs55 deleted the window-write branch February 21, 2023 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants