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

UPSTREAM: fix ldapserver test panic #4168

Merged
merged 1 commit into from
Aug 17, 2015
Merged

UPSTREAM: fix ldapserver test panic #4168

merged 1 commit into from
Aug 17, 2015

Conversation

liggitt
Copy link
Contributor

@liggitt liggitt commented Aug 14, 2015

Upstream PR: vjeantet/ldapserver#15
Fixes #4066

@smarterclayton
Copy link
Contributor

Is there multithreaded access to that close?

@liggitt
Copy link
Contributor Author

liggitt commented Aug 14, 2015

calling twice will still cause problems. this is just fixing the case where no one was interested in a callback, and Abandon called close on a nil channel anyway. this is only used in the integration test, I have a TODO to revisit

@smarterclayton
Copy link
Contributor

Is this WIP for a reason?

@liggitt
Copy link
Contributor Author

liggitt commented Aug 14, 2015

was going to see if I could get a quick merge upstream to avoid breaking godeps

@liggitt liggitt changed the title WIP - fix ldapserver test panic UPSTREAM: fix ldapserver test panic Aug 17, 2015
@liggitt
Copy link
Contributor Author

liggitt commented Aug 17, 2015

[test]

@liggitt
Copy link
Contributor Author

liggitt commented Aug 17, 2015

@smarterclayton no movement upstream, this is fine to merge now. not sure what to do with godeps to help prevent stompage of patches on less-scrutinized vendered packages

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/4186/)

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to de466c4

@smarterclayton
Copy link
Contributor

Other than a test case that validates it, not much. Better process around rebase.

@smarterclayton
Copy link
Contributor

[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/2995/) (Image: devenv-fedora_2173)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to de466c4

openshift-bot pushed a commit that referenced this pull request Aug 17, 2015
@openshift-bot openshift-bot merged commit 9a6dc91 into openshift:master Aug 17, 2015
@liggitt liggitt deleted the ldapserver_panic branch August 18, 2015 04:28
@stevekuznetsov
Copy link
Contributor

@liggitt this was accepted upstream, should we bump and remove this patch?

@liggitt
Copy link
Contributor Author

liggitt commented Aug 21, 2015

sure

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.

4 participants