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

Comment to provoke review of suspect code #982

Closed
wants to merge 1 commit into from
Closed

Comment to provoke review of suspect code #982

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/lb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ int zmq::lb_t::sendpipe (msg_t *msg_, pipe_t **pipe_)
more = msg_->flags () & msg_t::more? true: false;
if (!more) {
pipes [current]->flush ();
current = (current + 1) % active;
current = (current + 1) % active; // in zmq 3.x this causes crash if the last pipe is terminated beween the
// the test for active == 0 at 109 and here as %(active==0) causes a crash.
Copy link
Member

Choose a reason for hiding this comment

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

A think we can safely assert that active is non zero here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Martin,

I do not know the zeromq code very well... and the current code not at all.

The question I raise is "Is 'active' modified and read in two different
threads?".
Because I am confident that in zeromq3-x it is modified in two threads and
a race to crash exists as a consequence.

On Fri, Apr 25, 2014 at 4:54 PM, Martin Hurton [email protected]:

In src/lb.cpp:

@@ -116,7 +116,8 @@ int zmq::lb_t::sendpipe (msg_t msg, pipe_t *pipe)
more = msg_->flags () & msg_t::more? true: false;
if (!more) {
pipes [current]->flush ();

  •    current = (current + 1) % active;

  •    current = (current + 1) % active; //  in zmq 3.x this causes crash if the last pipe is terminated beween the

  •                                      //    the test for active == 0 at 109  and here as %(active==0) causes a crash.

A think we can safely assert that active is non zero here.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/982/files#r12002719
.

Copy link
Member

Choose a reason for hiding this comment

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

The code should be executed only in one thread, provided the application does not invoke socket operation in two different threads. That's the assumption of the library.

Copy link
Member

Choose a reason for hiding this comment

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

One of the threads involved here is the IO thread in the zmq context.

}

// Detach the message from the data buffer.
Expand Down