Skip to content

Commit

Permalink
Fix exit deadlock due to wrong socket-context dtor order
Browse files Browse the repository at this point in the history
  • Loading branch information
Stefan Eilemann authored and Stefan Eilemann committed Jul 20, 2017
1 parent b64eb3f commit f41d03d
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 15 deletions.
1 change: 0 additions & 1 deletion tests/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ std::ostream& operator<<(std::ostream& os, const Echo& echo)
class Empty : public servus::Serializable
{
public:
Empty() = default;
static std::string TYPENAME() { return "zeroeq::test::Empty"; }
static servus::uint128_t IDENTIFIER()
{
Expand Down
4 changes: 2 additions & 2 deletions tests/reqRep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ BOOST_AUTO_TEST_CASE(empty_reqrep)
{
zeroeq::Server server(zeroeq::NULL_SESSION);
zeroeq::Client client({server.getURI()});
const test::Empty reply;
const test::Empty reply{};

bool serverHandled = false;
std::thread thread([&] { serverHandled = runOnce(server, {}, reply); });
Expand Down Expand Up @@ -191,7 +191,7 @@ BOOST_AUTO_TEST_CASE(unhandled_request)
{
zeroeq::Server server(zeroeq::NULL_SESSION);
zeroeq::Client client({server.getURI()});
const test::Empty reply;
const test::Empty reply{};

bool serverHandled = false;
std::thread thread([&] { serverHandled = runOnce(server, {}, reply); });
Expand Down
8 changes: 2 additions & 6 deletions zeroeq/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include "detail/browser.h"
#include "detail/common.h"
#include "detail/context.h"

#include <servus/servus.h>
#include <unordered_map>
Expand All @@ -20,8 +19,7 @@ class Client::Impl : public detail::Browser
Impl(const std::string& session)
: Browser(SERVER_SERVICE,
session == DEFAULT_SESSION ? getDefaultAppSession() : session)
, _context(detail::getContext())
, _servers(zmq_socket(_context.get(), ZMQ_DEALER),
, _servers(zmq_socket(getContext(), ZMQ_DEALER),
[](void* s) { ::zmq_close(s); })
{
if (session == zeroeq::NULL_SESSION || session.empty())
Expand All @@ -31,8 +29,7 @@ class Client::Impl : public detail::Browser

Impl(const URIs& uris)
: Browser(SERVER_SERVICE, {})
, _context(detail::getContext())
, _servers(zmq_socket(_context.get(), ZMQ_DEALER),
, _servers(zmq_socket(getContext(), ZMQ_DEALER),
[](void* s) { ::zmq_close(s); })
{
for (const auto& uri : uris)
Expand Down Expand Up @@ -158,7 +155,6 @@ class Client::Impl : public detail::Browser
return more;
}

zmq::ContextPtr _context;
zmq::SocketPtr _servers;
std::unordered_map<uint128_t, ReplyFunc> _handlers;
};
Expand Down
5 changes: 4 additions & 1 deletion zeroeq/detail/browser.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "common.h"
#include "constants.h"
#include "context.h"
#include "socket.h"

#include "../log.h"
Expand All @@ -24,6 +25,7 @@ class Browser
Browser(const std::string& service, const std::string session)
: _servus(service)
, _session(session)
, _context(detail::getContext())
{
if (_session == zeroeq::NULL_SESSION || session.empty())
return;
Expand All @@ -43,7 +45,6 @@ class Browser
}

const std::string& getSession() const { return _session; }

void update()
{
if (!_servus.isBrowsing())
Expand Down Expand Up @@ -85,6 +86,7 @@ class Browser
protected:
using SocketMap = std::map<std::string, zmq::SocketPtr>;

void* getContext() { return _context.get(); }
const SocketMap& getSockets() { return _sockets; }
bool addConnection(const std::string& zmqURI, zmq::SocketPtr socket)
{
Expand All @@ -104,6 +106,7 @@ class Browser
servus::Servus _servus;
const std::string _session;

zmq::ContextPtr _context;
SocketMap _sockets;
std::vector<detail::Socket> _entries;

Expand Down
6 changes: 1 addition & 5 deletions zeroeq/subscriber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "detail/byteswap.h"
#include "detail/common.h"
#include "detail/constants.h"
#include "detail/context.h"
#include "detail/sender.h"
#include "detail/socket.h"
#include "log.h"
Expand All @@ -32,7 +31,6 @@ class Subscriber::Impl : public detail::Browser
: Browser(PUBLISHER_SERVICE, session == DEFAULT_SESSION
? getDefaultUserSession()
: session)
, _context(detail::getContext())
, _selfInstance(detail::Sender::getUUID())
{
if (session == zeroeq::NULL_SESSION || session.empty())
Expand All @@ -42,7 +40,6 @@ class Subscriber::Impl : public detail::Browser

Impl(const URI& uri)
: Browser(PUBLISHER_SERVICE, {})
, _context(detail::getContext())
, _selfInstance(detail::Sender::getUUID())
{
if (uri.getScheme() == DEFAULT_SCHEMA &&
Expand Down Expand Up @@ -139,7 +136,7 @@ class Subscriber::Impl : public detail::Browser
if (instance == _selfInstance)
return true;

zmq::SocketPtr socket(zmq_socket(_context.get(), ZMQ_SUB),
zmq::SocketPtr socket(zmq_socket(getContext(), ZMQ_SUB),
[](void* s) { ::zmq_close(s); });
const int hwm = 0;
zmq_setsockopt(socket.get(), ZMQ_RCVHWM, &hwm, sizeof(hwm));
Expand Down Expand Up @@ -171,7 +168,6 @@ class Subscriber::Impl : public detail::Browser
}

private:
zmq::ContextPtr _context;
typedef std::map<uint128_t, EventPayloadFunc> EventFuncMap;
EventFuncMap _eventFuncs;

Expand Down

0 comments on commit f41d03d

Please sign in to comment.