Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix some goroutine leaks in commands #4413

Merged
merged 2 commits into from
Nov 22, 2017
Merged

Fix some goroutine leaks in commands #4413

merged 2 commits into from
Nov 22, 2017

Conversation

Stebalien
Copy link
Member

There's a lot left but I'll just make an issue and advertise it on IRC.

@Stebalien
fix goroutine leaks in DHT commands
016b005 
License: MIT
Signed-off-by: Steven Allen <[email protected]>
@ghost ghost assigned Stebalien Nov 21, 2017
@ghost ghost added the status/in-progress In progress label Nov 21, 2017
@Stebalien
Copy link
Member Author

Question: Should we be calling res.SetError(req.Context().Err(), cmdkit.ErrNormal) when the context is canceled or can we just bail (what I'm doing here).

@Stebalien Stebalien requested a review from keks November 21, 2017 23:41
@Stebalien
Copy link
Member Author

See #4414

@Stebalien Stebalien mentioned this pull request Nov 21, 2017
Closed
12 tasks
@Stebalien
fix goroutine leaks in repo commands
2997f48 
License: MIT
Signed-off-by: Steven Allen <[email protected]>
@whyrusleeping whyrusleeping merged commit 1f2247c into master Nov 22, 2017
@whyrusleeping whyrusleeping deleted the fix/cmd-goroutine-leaks branch November 22, 2017 05:26
@ghost ghost removed the status/in-progress In progress label Nov 22, 2017
lgarron added a commit to lgarron/go-ipfs that referenced this pull request Jan 18, 2018
@lgarron
Avoid closing channels in commands if the context has errored (e.g. t…
fd831be 
…imed out).

Previously, the following could happen:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and closes the channel.
- Consumer sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and DOES NOT close the
  channel.
- Consumer sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.
@lgarron lgarron mentioned this pull request Jan 18, 2018
Closed
lgarron added a commit to lgarron/go-ipfs that referenced this pull request Jan 18, 2018
@lgarron
Avoid closing channels in commands if the context has errored (e.g. t…
a386712 
…imed out).

Previously, the following could happen:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and closes the channel.
- Consumer sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and DOES NOT close the
  channel.
- Consumer sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.
lgarron added a commit to lgarron/go-ipfs that referenced this pull request Jan 18, 2018
@lgarron
Avoid closing channels in commands if the context has errored (e.g. t…
f717cc4 
…imed out).

Previously, the following could happen:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and closes the channel.
- Consumer sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and DOES NOT close the
  channel.
- Consumer sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.

License: MIT
Signed-off-by: Lucas Garron <[email protected]>
lgarron added a commit to lgarron/go-ipfs that referenced this pull request Jan 18, 2018
@lgarron
Avoid closing channels in commands if the context has errored (e.g. t…
ecbb171 
…imed out).

Previously, the following could happen:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and closes the channel.
- Consumer sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and DOES NOT close the
  channel.
- Consumer sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.

License: MIT
Signed-off-by: Lucas Garron <[email protected]>
lgarron added a commit to lgarron/go-ipfs that referenced this pull request Jan 18, 2018
@lgarron
Avoid closing channels in commands if the context has errored (e.g. t…
e8fbc1b 
…imed out).

Previously, the following could happen:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and closes the channel.
- Consumer sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and DOES NOT close the
  channel.
- Consumer sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.

License: MIT
Signed-off-by: Lucas Garron <[email protected]>
lgarron added a commit to lgarron/go-ipfs that referenced this pull request Jan 18, 2018
@lgarron
Avoid closing channels in commands if the context has errored (e.g. t…
cffdbc9 
…imed out).

Previously, the following could happen:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and closes the channel.
- Consumer sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and DOES NOT close the
  channel.
- Consumer sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.

License: MIT
Signed-off-by: Lucas Garron <[email protected]>
lgarron added a commit to lgarron/go-ipfs that referenced this pull request Jan 18, 2018
@lgarron
Avoid closing channels in commands if the context has errored (e.g. t…
f57391e 
…imed out).

Previously, the following could happen:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and closes the channel.
- Consumer sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Producer creates channel.
- Consumer reads from channel.
- Producer observes that the context has errored, and DOES NOT close the
  channel.
- Consumer sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.

License: MIT
Signed-off-by: Lucas Garron <[email protected]>
lgarron added a commit to lgarron/go-ipfs that referenced this pull request Jan 18, 2018
@lgarron
Avoid closing channels in commands if the context has errored (e.g. t…
5a20af5 
…imed out).

Previously, the following could happen:
- Sender creates channel.
- Receiver reads from channel.
- Sender observes that the context has errored, and closes the channel.
- Receiver sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Sender creates channel.
- Receiver reads from channel.
- Sender observes that the context has errored, and DOES NOT close the
  channel.
- Receiver sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.

License: MIT
Signed-off-by: Lucas Garron <[email protected]>
lgarron added a commit to lgarron/go-ipfs that referenced this pull request Jan 18, 2018
@lgarron
Avoid closing channels in commands if the context has errored (e.g. t…
ac3790d 
…imed out).

Previously, the following could happen:
- Sender creates channel.
- Receiver reads from channel.
- Sender observes that the context has errored, and closes the channel.
- Receiver sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Sender creates channel.
- Receiver reads from channel.
- Sender observes that the context has errored, and DOES NOT close the
  channel.
- Receiver sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.

License: MIT
Signed-off-by: Lucas Garron <[email protected]>
lgarron added a commit to lgarron/go-ipfs that referenced this pull request Jan 18, 2018
@lgarron
Avoid closing channels in commands if the context has errored (e.g. t…
e215206 
…imed out).

Previously, the following could happen:
- Sender creates channel.
- Receiver reads from channel.
- Sender observes that the context has errored, and closes the channel.
- Receiver sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Sender creates channel.
- Receiver reads from channel.
- Sender observes that the context has errored, and DOES NOT close the
  channel.
- Receiver sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.

License: MIT
Signed-off-by: Lucas Garron <[email protected]>
lgarron added a commit to lgarron/go-ipfs that referenced this pull request Jan 18, 2018
@lgarron
Avoid closing channels in commands if the context has errored (e.g. t…
1389cb4 
…imed out).

Previously, the following could happen:
- Sender creates channel.
- Receiver reads from channel.
- Sender observes that the context has errored, and closes the channel.
- Receiver sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Sender creates channel.
- Receiver reads from channel.
- Sender observes that the context has errored, and DOES NOT close the
  channel.
- Receiver sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.

License: MIT
Signed-off-by: Lucas Garron <[email protected]>
lgarron added a commit to lgarron/go-ipfs that referenced this pull request Jan 18, 2018
@lgarron
Avoid closing channels in commands if the context has errored (e.g. t…
3736e95 
…imed out).

Previously, the following could happen:
- Sender creates channel.
- Receiver reads from channel.
- Sender observes that the context has errored, and closes the channel.
- Receiver sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Sender creates channel.
- Receiver reads from channel.
- Sender observes that the context has errored, and DOES NOT close the
  channel.
- Receiver sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.

License: MIT
Signed-off-by: Lucas Garron <[email protected]>
Stebalien pushed a commit to lgarron/go-ipfs that referenced this pull request Jan 26, 2018
@lgarron @Stebalien
Avoid closing channels in commands if the context has errored (e.g. t…
304efa3 
…imed out).

Previously, the following could happen:
- Sender creates channel.
- Receiver reads from channel.
- Sender observes that the context has errored, and closes the channel.
- Receiver sees the closed channel before seeing the context error, and assumes
  that command completed without a context error.

This change ensures the following:
- Sender creates channel.
- Receiver reads from channel.
- Sender observes that the context has errored, and DOES NOT close the
  channel.
- Receiver sees that the context has errored, and handles the channel
  appropriately.

Note that per the definition of context [1], a context is "done" iff it has not
"errored" (i.e. ctx.Err() is non-nil).

[1] https://godoc.org/context#Context

Updates call sites touched by ipfs#4413 and addresses ipfs#4414.

License: MIT
Signed-off-by: Lucas Garron <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whyrusleeping whyrusleeping whyrusleeping approved these changes

@keks keks Awaiting requested review from keks

Assignees

@Stebalien Stebalien

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Stebalien @whyrusleeping