Skip to content

Fix shutdown bugs by holding loggers in shared_ptrs #539

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions test/connection/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,14 @@ struct debug_config_client : public websocketpp::config::core {
};

struct connection_setup {
connection_setup(bool p_is_server) : c(p_is_server, "", alog, elog, rng) {}
connection_setup(bool p_is_server)
: alog(websocketpp::lib::make_shared<stub_config::alog_type>())
, elog(websocketpp::lib::make_shared<stub_config::elog_type>())
, c(p_is_server, "", alog, elog, rng) {}

websocketpp::lib::error_code ec;
stub_config::alog_type alog;
stub_config::elog_type elog;
websocketpp::lib::shared_ptr<stub_config::alog_type> alog;
websocketpp::lib::shared_ptr<stub_config::elog_type> elog;
stub_config::rng_type rng;
websocketpp::connection<stub_config> c;
};
Expand Down
12 changes: 7 additions & 5 deletions test/transport/asio/timers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ context_ptr on_tls_init(websocketpp::connection_hdl) {
struct mock_con: public websocketpp::transport::asio::connection<config> {
typedef websocketpp::transport::asio::connection<config> base;

mock_con(bool a, config::alog_type& b, config::elog_type& c) : base(a,b,c) {}
mock_con(bool a, const websocketpp::lib::shared_ptr<config::alog_type>& b,
const websocketpp::lib::shared_ptr<config::elog_type>& c)
: base(a,b,c) {}

void start() {
base::init(websocketpp::lib::bind(&mock_con::handle_start,this,
Expand All @@ -139,8 +141,8 @@ struct mock_endpoint : public websocketpp::transport::asio::endpoint<config> {
typedef websocketpp::transport::asio::endpoint<config> base;

mock_endpoint() {
alog.set_channels(websocketpp::log::alevel::all);
base::init_logging(&alog,&elog);
alog->set_channels(websocketpp::log::alevel::all);
base::init_logging(alog,elog);
init_asio();
}

Expand Down Expand Up @@ -170,8 +172,8 @@ struct mock_endpoint : public websocketpp::transport::asio::endpoint<config> {
}

connection_ptr m_con;
config::alog_type alog;
config::elog_type elog;
websocketpp::lib::shared_ptr<config::alog_type> alog;
websocketpp::lib::shared_ptr<config::elog_type> elog;
};

BOOST_AUTO_TEST_CASE( tls_handshake_timeout ) {
Expand Down
6 changes: 3 additions & 3 deletions test/transport/iostream/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ struct stub_con : public iostream_con {
typedef websocketpp::lib::shared_ptr<type> ptr;
typedef iostream_con::timer_ptr timer_ptr;

stub_con(bool is_server, config::alog_type & a, config::elog_type & e)
stub_con(bool is_server, const websocketpp::lib::shared_ptr<config::alog_type>& a, const websocketpp::lib::shared_ptr<config::elog_type>& e)
: iostream_con(is_server,a,e)
// Set the error to a known code that is unused by the library
// This way we can easily confirm that the handler was run at all.
Expand Down Expand Up @@ -164,8 +164,8 @@ struct stub_con : public iostream_con {
};

// Stubs
config::alog_type alogger;
config::elog_type elogger;
websocketpp::lib::shared_ptr<config::alog_type> alogger = websocketpp::lib::make_shared<config::alog_type>();
websocketpp::lib::shared_ptr<config::elog_type> elogger = websocketpp::lib::make_shared<config::elog_type>();

BOOST_AUTO_TEST_CASE( const_methods ) {
iostream_con::ptr con(new iostream_con(true,alogger,elogger));
Expand Down
1 change: 1 addition & 0 deletions websocketpp/config/core.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@

// Loggers
#include <websocketpp/logger/basic.hpp>
#include <websocketpp/logger/levels.hpp>

// RNG
#include <websocketpp/random/none.hpp>
Expand Down
12 changes: 6 additions & 6 deletions websocketpp/connection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,8 @@ class connection
};
public:

explicit connection(bool p_is_server, std::string const & ua, alog_type& alog,
elog_type& elog, rng_type & rng)
explicit connection(bool p_is_server, std::string const & ua, const lib::shared_ptr<alog_type>& alog,
const lib::shared_ptr<elog_type>& elog, rng_type & rng)
: transport_con_type(p_is_server, alog, elog)
, m_handle_read_frame(lib::bind(
&type::handle_read_frame,
Expand Down Expand Up @@ -329,7 +329,7 @@ class connection
, m_http_state(session::http_state::init)
, m_was_clean(false)
{
m_alog.write(log::alevel::devel,"connection constructor");
m_alog->write(log::alevel::devel,"connection constructor");
}

/// Get a shared pointer to this component
Expand Down Expand Up @@ -1486,7 +1486,7 @@ class connection
void log_err(log::level l, char const * msg, error_type const & ec) {
std::stringstream s;
s << msg << " error: " << ec << " (" << ec.message() << ")";
m_elog.write(l, s.str());
m_elog->write(l, s.str());
}

// internal handler functions
Expand Down Expand Up @@ -1603,8 +1603,8 @@ class connection
std::vector<std::string> m_requested_subprotocols;

bool const m_is_server;
alog_type& m_alog;
elog_type& m_elog;
const lib::shared_ptr<alog_type> m_alog;
const lib::shared_ptr<elog_type> m_elog;

rng_type & m_rng;

Expand Down
48 changes: 24 additions & 24 deletions websocketpp/endpoint.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ class endpoint : public config::transport_type, public config::endpoint_base {
//friend connection;

explicit endpoint(bool p_is_server)
: m_alog(config::alog_level, log::channel_type_hint::access)
, m_elog(config::elog_level, log::channel_type_hint::error)
: m_alog(new alog_type(config::alog_level, log::channel_type_hint::access))
, m_elog(new elog_type(config::elog_level, log::channel_type_hint::error))
, m_user_agent(::websocketpp::user_agent)
, m_open_handshake_timeout_dur(config::timeout_open_handshake)
, m_close_handshake_timeout_dur(config::timeout_close_handshake)
Expand All @@ -99,12 +99,12 @@ class endpoint : public config::transport_type, public config::endpoint_base {
, m_max_http_body_size(config::max_http_body_size)
, m_is_server(p_is_server)
{
m_alog.set_channels(config::alog_level);
m_elog.set_channels(config::elog_level);
m_alog->set_channels(config::alog_level);
m_elog->set_channels(config::elog_level);

m_alog.write(log::alevel::devel, "endpoint constructor");
m_alog->write(log::alevel::devel, "endpoint constructor");

transport_type::init_logging(&m_alog, &m_elog);
transport_type::init_logging(m_alog, m_elog);
}


Expand Down Expand Up @@ -218,7 +218,7 @@ class endpoint : public config::transport_type, public config::endpoint_base {
* @param channels The channel value(s) to set
*/
void set_access_channels(log::level channels) {
m_alog.set_channels(channels);
m_alog->set_channels(channels);
}

/// Clear Access logging channels
Expand All @@ -229,7 +229,7 @@ class endpoint : public config::transport_type, public config::endpoint_base {
* @param channels The channel value(s) to clear
*/
void clear_access_channels(log::level channels) {
m_alog.clear_channels(channels);
m_alog->clear_channels(channels);
}

/// Set Error logging channel
Expand All @@ -240,7 +240,7 @@ class endpoint : public config::transport_type, public config::endpoint_base {
* @param channels The channel value(s) to set
*/
void set_error_channels(log::level channels) {
m_elog.set_channels(channels);
m_elog->set_channels(channels);
}

/// Clear Error logging channels
Expand All @@ -251,76 +251,76 @@ class endpoint : public config::transport_type, public config::endpoint_base {
* @param channels The channel value(s) to clear
*/
void clear_error_channels(log::level channels) {
m_elog.clear_channels(channels);
m_elog->clear_channels(channels);
}

/// Get reference to access logger
/**
* @return A reference to the access logger
*/
alog_type & get_alog() {
return m_alog;
return *m_alog;
}

/// Get reference to error logger
/**
* @return A reference to the error logger
*/
elog_type & get_elog() {
return m_elog;
return *m_elog;
}

/*************************/
/* Set Handler functions */
/*************************/

void set_open_handler(open_handler h) {
m_alog.write(log::alevel::devel,"set_open_handler");
m_alog->write(log::alevel::devel,"set_open_handler");
scoped_lock_type guard(m_mutex);
m_open_handler = h;
}
void set_close_handler(close_handler h) {
m_alog.write(log::alevel::devel,"set_close_handler");
m_alog->write(log::alevel::devel,"set_close_handler");
scoped_lock_type guard(m_mutex);
m_close_handler = h;
}
void set_fail_handler(fail_handler h) {
m_alog.write(log::alevel::devel,"set_fail_handler");
m_alog->write(log::alevel::devel,"set_fail_handler");
scoped_lock_type guard(m_mutex);
m_fail_handler = h;
}
void set_ping_handler(ping_handler h) {
m_alog.write(log::alevel::devel,"set_ping_handler");
m_alog->write(log::alevel::devel,"set_ping_handler");
scoped_lock_type guard(m_mutex);
m_ping_handler = h;
}
void set_pong_handler(pong_handler h) {
m_alog.write(log::alevel::devel,"set_pong_handler");
m_alog->write(log::alevel::devel,"set_pong_handler");
scoped_lock_type guard(m_mutex);
m_pong_handler = h;
}
void set_pong_timeout_handler(pong_timeout_handler h) {
m_alog.write(log::alevel::devel,"set_pong_timeout_handler");
m_alog->write(log::alevel::devel,"set_pong_timeout_handler");
scoped_lock_type guard(m_mutex);
m_pong_timeout_handler = h;
}
void set_interrupt_handler(interrupt_handler h) {
m_alog.write(log::alevel::devel,"set_interrupt_handler");
m_alog->write(log::alevel::devel,"set_interrupt_handler");
scoped_lock_type guard(m_mutex);
m_interrupt_handler = h;
}
void set_http_handler(http_handler h) {
m_alog.write(log::alevel::devel,"set_http_handler");
m_alog->write(log::alevel::devel,"set_http_handler");
scoped_lock_type guard(m_mutex);
m_http_handler = h;
}
void set_validate_handler(validate_handler h) {
m_alog.write(log::alevel::devel,"set_validate_handler");
m_alog->write(log::alevel::devel,"set_validate_handler");
scoped_lock_type guard(m_mutex);
m_validate_handler = h;
}
void set_message_handler(message_handler h) {
m_alog.write(log::alevel::devel,"set_message_handler");
m_alog->write(log::alevel::devel,"set_message_handler");
scoped_lock_type guard(m_mutex);
m_message_handler = h;
}
Expand Down Expand Up @@ -661,8 +661,8 @@ class endpoint : public config::transport_type, public config::endpoint_base {
protected:
connection_ptr create_connection();

alog_type m_alog;
elog_type m_elog;
lib::shared_ptr<alog_type> m_alog;
lib::shared_ptr<elog_type> m_elog;
private:
// dynamic settings
std::string m_user_agent;
Expand Down
Loading