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

v2.x: coll/libnbc: fix race condition with multi threaded apps #2441

Merged
merged 2 commits into from
Dec 5, 2016

Conversation

ggouaillardet
Copy link
Contributor

protect the mca_coll_libnbc_component.active_requests list with
the new mca_coll_libnbc_component.lock mutex.

Thanks Jie Hu for the report

Signed-off-by: Gilles Gouaillardet gilles@rist.or.jp

(cherry picked from commit 2c94a3a)

@ggouaillardet ggouaillardet added this to the v2.1.0 milestone Nov 21, 2016
@ggouaillardet ggouaillardet changed the title coll/libnbc: fix race condition with multi threaded apps v2.x: coll/libnbc: fix race condition with multi threaded apps Nov 21, 2016
@hppritcha
Copy link
Member

hppritcha commented Nov 22, 2016

@bosilca can you review when you get a chance? Also #2442 #2443

@jjhursey
Copy link
Member

@ggouaillardet I'm curious why you didn't just reuse mca_coll_libnbc_component.progress_lock? Since the progress function is already protected by that lock then you would not have to grab another lock in there, but instead just protect the opal_list_append.

@bosilca
Copy link
Member

bosilca commented Nov 22, 2016

There is an ongoing discussion about this on #2443.

protect the mca_coll_libnbc_component.active_requests list with
the new mca_coll_libnbc_component.lock mutex.

Thanks Jie Hu for the report

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>

(cherry picked from commit open-mpi/ompi@2c94a3a)
no code change

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>

(cherry picked from commit open-mpi/ompi@1509816)
Copy link
Member

@jjhursey jjhursey left a comment

Choose a reason for hiding this comment

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

Per discussion of same patch for PR #2443 - this looks good. Thanks!

@jsquyres jsquyres merged commit 71efb64 into open-mpi:v2.x Dec 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants