Skip to content

Make onclose an option.#35

Merged
crosbymichael merged 1 commit intocontainerd:masterfrom
Random-Liu:make-on-close-an-options
Apr 11, 2019
Merged

Make onclose an option.#35
crosbymichael merged 1 commit intocontainerd:masterfrom
Random-Liu:make-on-close-an-options

Conversation

@Random-Liu
Copy link
Copy Markdown
Member

Make OnClose an create option instead of a separate function.

NewClient starts a goroutine, which calls closeFunc before exit.

Setting closeFunc after NewClient separately introduces a race condition:

  1. The race condition can cause unexpected behavior, e.g. the function is not fully copied before being invoked;
  2. There is no way to guarantee a closeFunc is called given that the connection may be gone before OnClose is set.

Signed-off-by: Lantao Liu lantaol@google.com

Signed-off-by: Lantao Liu <lantaol@google.com>
@crosbymichael
Copy link
Copy Markdown
Member

LGTM

@crosbymichael crosbymichael merged commit 699c4e4 into containerd:master Apr 11, 2019
@Random-Liu Random-Liu deleted the make-on-close-an-options branch April 11, 2019 18:45
thaJeztah added a commit to thaJeztah/containerd that referenced this pull request Apr 27, 2019
full diff: containerd/ttrpc@f02858b...699c4e4

- containerd/ttrpc#33 Fix returns error message
- containerd/ttrpc#35 Make onclose an option

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/containerd that referenced this pull request Apr 29, 2019
full diff: containerd/ttrpc@f02858b...699c4e4

- containerd/ttrpc#33 Fix returns error message
- containerd/ttrpc#35 Make onclose an option

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 8c5779c)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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