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.

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

bitswap/workers: fix proc / ctx wiring #1655

Merged
merged 1 commit into from
Sep 5, 2015
Merged

bitswap/workers: fix proc / ctx wiring #1655

merged 1 commit into from
Sep 5, 2015

Conversation

jbenet
Copy link
Member

@jbenet jbenet commented Sep 5, 2015

This commit changes the order of the proc/ctx wiring, to ensure
that the proc has been setup correctly before exiting.

License: MIT
Signed-off-by: Juan Batiz-Benet [email protected]

This commit changes the order of the proc/ctx wiring, to ensure
that the proc has been setup correctly before exiting.

License: MIT
Signed-off-by: Juan Batiz-Benet <[email protected]>
@jbenet jbenet added the status/in-progress In progress label Sep 5, 2015
@jbenet
Copy link
Member Author

jbenet commented Sep 5, 2015

@whyrusleeping i fixed that proc bug

jbenet added a commit that referenced this pull request Sep 5, 2015
bitswap/workers: fix proc / ctx wiring
@jbenet jbenet merged commit f1e86ec into master Sep 5, 2015
@jbenet jbenet removed the status/in-progress In progress label Sep 5, 2015
@whyrusleeping
Copy link
Member

tests???

@jbenet jbenet deleted the procfix branch September 5, 2015 02:53
@jbenet
Copy link
Member Author

jbenet commented Sep 5, 2015

@whyrusleeping in the prev config, it's an odd race. i dont know of a reliable way to repro other than run travis a ton of times.

@jbenet
Copy link
Member Author

jbenet commented Sep 5, 2015

(happy to add something if you have a good suggestion)

@whyrusleeping
Copy link
Member

yeah, i'm not really sure. yay race conditionssss

<-px.Closing() // process closes first
cancelFunc()
}()
procctx.CloseAfterContext(px, ctx) // parent cancelled first
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason that bitswap.New has WithCancel but dht.NewDHT doesn't?

@jbenet jbenet mentioned this pull request Sep 7, 2015
42 tasks
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