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

Improve API safety by giving Socket a lifetime #65

Closed
wants to merge 1 commit into from

Conversation

jdpage
Copy link

@jdpage jdpage commented Jun 4, 2015

Socket now has a lifetime parameter which allows the compiler ensure that
sockets last at least as long as their contexts. In order to make this
practical, Context::socket() is now &self instead of &mut self. ZMQ contexts are
already thread-safe, so this kind of aliasing should be okay.

Socket now has a lifetime parameter which allows the compiler ensure that
sockets last at least as long as their contexts. In order to make this
practical, Context::socket() is now &self instead of &mut self. ZMQ contexts are
already thread-safe, so this kind of aliasing should be okay.

pull_socket.bind("inproc://server-pull").unwrap();
push_socket.bind("inproc://server-push").unwrap();

Copy link
Author

Choose a reason for hiding this comment

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

By using Arc for the context and moving the socket creation into the thread, the compiler automatically pulls the Arc pointer into the thread context, ensuring it lives long enough.

@alexkamp
Copy link

It would be important to merge this, because otherwise the suggested version on how to share context between threads does not work (see #64).

I see that the TravisCI build failed, however, the log says "build stopped" while it is installing rust. So it does not seem that this Pull Request is the problem, it seems that something about the configuration of Travis is.

@jdpage
Copy link
Author

jdpage commented Oct 20, 2015

@alexkamp If I recall correctly, TravisCI was having trouble with Rust builds at the time, and I think one of the project owners would have to be the one to retrigger the build. However, this has been sitting around so long that it has merge conflicts, so at this point it'd probably be better if someone brought this branch up to speed and re-did the PR, which should cause a new TravisCI build to start.

@rotty
Copy link
Collaborator

rotty commented Nov 16, 2016

The underlying issue should have been resolved by merging #96, although this keeps the context alive as long as there are sockets open (using an Arc).

@rotty rotty closed this Nov 16, 2016
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.

None yet

3 participants