nghttpx: Fix bug that mruby is incorrectly shared between backends

Previously, mruby context is wrongly shared by multiple patterns if
the underlying SharedDownstreamAddr is shared by multiple
DownstreamAddrGroups.  This commit fixes it.
This commit is contained in:
Tatsuhiro Tsujikawa 2019-09-16 22:25:06 +09:00
parent f8933fe504
commit fe8946ddc7
5 changed files with 27 additions and 24 deletions

View File

@ -193,7 +193,7 @@ Downstream::~Downstream() {
if (dconn_) { if (dconn_) {
const auto &group = dconn_->get_downstream_addr_group(); const auto &group = dconn_->get_downstream_addr_group();
if (group) { if (group) {
const auto &mruby_ctx = group->mruby_ctx; const auto &mruby_ctx = group->shared_addr->mruby_ctx;
mruby_ctx->delete_downstream(this); mruby_ctx->delete_downstream(this);
} }
} }
@ -231,7 +231,7 @@ void Downstream::detach_downstream_connection() {
#ifdef HAVE_MRUBY #ifdef HAVE_MRUBY
const auto &group = dconn_->get_downstream_addr_group(); const auto &group = dconn_->get_downstream_addr_group();
if (group) { if (group) {
const auto &mruby_ctx = group->mruby_ctx; const auto &mruby_ctx = group->shared_addr->mruby_ctx;
mruby_ctx->delete_downstream(this); mruby_ctx->delete_downstream(this);
} }
#endif // HAVE_MRUBY #endif // HAVE_MRUBY
@ -256,7 +256,7 @@ std::unique_ptr<DownstreamConnection> Downstream::pop_downstream_connection() {
const auto &group = dconn_->get_downstream_addr_group(); const auto &group = dconn_->get_downstream_addr_group();
if (group) { if (group) {
const auto &mruby_ctx = group->mruby_ctx; const auto &mruby_ctx = group->shared_addr->mruby_ctx;
mruby_ctx->delete_downstream(this); mruby_ctx->delete_downstream(this);
} }
#endif // HAVE_MRUBY #endif // HAVE_MRUBY

View File

@ -492,7 +492,7 @@ void Http2Upstream::initiate_downstream(Downstream *downstream) {
#ifdef HAVE_MRUBY #ifdef HAVE_MRUBY
const auto &group = dconn_ptr->get_downstream_addr_group(); const auto &group = dconn_ptr->get_downstream_addr_group();
if (group) { if (group) {
const auto &mruby_ctx = group->mruby_ctx; const auto &mruby_ctx = group->shared_addr->mruby_ctx;
if (mruby_ctx->run_on_request_proc(downstream) != 0) { if (mruby_ctx->run_on_request_proc(downstream) != 0) {
if (error_reply(downstream, 500) != 0) { if (error_reply(downstream, 500) != 0) {
rst_stream(downstream, NGHTTP2_INTERNAL_ERROR); rst_stream(downstream, NGHTTP2_INTERNAL_ERROR);
@ -1665,7 +1665,7 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) {
auto dconn = downstream->get_downstream_connection(); auto dconn = downstream->get_downstream_connection();
const auto &group = dconn->get_downstream_addr_group(); const auto &group = dconn->get_downstream_addr_group();
if (group) { if (group) {
const auto &dmruby_ctx = group->mruby_ctx; const auto &dmruby_ctx = group->shared_addr->mruby_ctx;
if (dmruby_ctx->run_on_response_proc(downstream) != 0) { if (dmruby_ctx->run_on_response_proc(downstream) != 0) {
if (error_reply(downstream, 500) != 0) { if (error_reply(downstream, 500) != 0) {

View File

@ -481,7 +481,7 @@ int htp_hdrs_completecb(llhttp_t *htp) {
#ifdef HAVE_MRUBY #ifdef HAVE_MRUBY
const auto &group = dconn_ptr->get_downstream_addr_group(); const auto &group = dconn_ptr->get_downstream_addr_group();
if (group) { if (group) {
const auto &dmruby_ctx = group->mruby_ctx; const auto &dmruby_ctx = group->shared_addr->mruby_ctx;
if (dmruby_ctx->run_on_request_proc(downstream) != 0) { if (dmruby_ctx->run_on_request_proc(downstream) != 0) {
resp.http_status = 500; resp.http_status = 500;
@ -1087,7 +1087,7 @@ int HttpsUpstream::on_downstream_header_complete(Downstream *downstream) {
assert(dconn); assert(dconn);
const auto &group = dconn->get_downstream_addr_group(); const auto &group = dconn->get_downstream_addr_group();
if (group) { if (group) {
const auto &dmruby_ctx = group->mruby_ctx; const auto &dmruby_ctx = group->shared_addr->mruby_ctx;
if (dmruby_ctx->run_on_response_proc(downstream) != 0) { if (dmruby_ctx->run_on_response_proc(downstream) != 0) {
error_reply(500); error_reply(500);

View File

@ -79,11 +79,12 @@ using DownstreamKey =
size_t, Proto, uint32_t, uint32_t, size_t, Proto, uint32_t, uint32_t,
uint32_t, bool, bool, bool, bool>>, uint32_t, bool, bool, bool, bool>>,
bool, SessionAffinity, StringRef, StringRef, bool, SessionAffinity, StringRef, StringRef,
SessionAffinityCookieSecure, int64_t, int64_t>; SessionAffinityCookieSecure, int64_t, int64_t, StringRef>;
namespace { namespace {
DownstreamKey create_downstream_key( DownstreamKey
const std::shared_ptr<SharedDownstreamAddr> &shared_addr) { create_downstream_key(const std::shared_ptr<SharedDownstreamAddr> &shared_addr,
const StringRef &mruby_file) {
DownstreamKey dkey; DownstreamKey dkey;
auto &addrs = std::get<0>(dkey); auto &addrs = std::get<0>(dkey);
@ -117,6 +118,7 @@ DownstreamKey create_downstream_key(
auto &timeout = shared_addr->timeout; auto &timeout = shared_addr->timeout;
std::get<6>(dkey) = timeout.read; std::get<6>(dkey) = timeout.read;
std::get<7>(dkey) = timeout.write; std::get<7>(dkey) = timeout.write;
std::get<8>(dkey) = mruby_file;
return dkey; return dkey;
} }
@ -231,16 +233,6 @@ void Worker::replace_downstream_config(
dst = std::make_shared<DownstreamAddrGroup>(); dst = std::make_shared<DownstreamAddrGroup>();
dst->pattern = dst->pattern =
ImmutableString{std::begin(src.pattern), std::end(src.pattern)}; ImmutableString{std::begin(src.pattern), std::end(src.pattern)};
#ifdef HAVE_MRUBY
auto mruby_ctx_it = shared_mruby_ctxs.find(src.mruby_file);
if (mruby_ctx_it == std::end(shared_mruby_ctxs)) {
dst->mruby_ctx = mruby::create_mruby_context(src.mruby_file);
assert(dst->mruby_ctx);
shared_mruby_ctxs.emplace(src.mruby_file, dst->mruby_ctx);
} else {
dst->mruby_ctx = (*mruby_ctx_it).second;
}
#endif // HAVE_MRUBY
auto shared_addr = std::make_shared<SharedDownstreamAddr>(); auto shared_addr = std::make_shared<SharedDownstreamAddr>();
@ -297,10 +289,21 @@ void Worker::replace_downstream_config(
loop_, cl_ssl_ctx_, this, &dst_addr, randgen_); loop_, cl_ssl_ctx_, this, &dst_addr, randgen_);
} }
#ifdef HAVE_MRUBY
auto mruby_ctx_it = shared_mruby_ctxs.find(src.mruby_file);
if (mruby_ctx_it == std::end(shared_mruby_ctxs)) {
shared_addr->mruby_ctx = mruby::create_mruby_context(src.mruby_file);
assert(shared_addr->mruby_ctx);
shared_mruby_ctxs.emplace(src.mruby_file, shared_addr->mruby_ctx);
} else {
shared_addr->mruby_ctx = (*mruby_ctx_it).second;
}
#endif // HAVE_MRUBY
// share the connection if patterns have the same set of backend // share the connection if patterns have the same set of backend
// addresses. // addresses.
auto dkey = create_downstream_key(shared_addr); auto dkey = create_downstream_key(shared_addr, src.mruby_file);
auto it = addr_groups_indexer.find(dkey); auto it = addr_groups_indexer.find(dkey);
if (it == std::end(addr_groups_indexer)) { if (it == std::end(addr_groups_indexer)) {

View File

@ -206,6 +206,9 @@ struct SharedDownstreamAddr {
// Bunch of session affinity hash. Only used if affinity == // Bunch of session affinity hash. Only used if affinity ==
// SessionAffinity::IP. // SessionAffinity::IP.
std::vector<AffinityHash> affinity_hash; std::vector<AffinityHash> affinity_hash;
#ifdef HAVE_MRUBY
std::shared_ptr<mruby::MRubyContext> mruby_ctx;
#endif // HAVE_MRUBY
// Configuration for session affinity // Configuration for session affinity
AffinityConfig affinity; AffinityConfig affinity;
// Session affinity // Session affinity
@ -230,9 +233,6 @@ struct DownstreamAddrGroup {
ImmutableString pattern; ImmutableString pattern;
std::shared_ptr<SharedDownstreamAddr> shared_addr; std::shared_ptr<SharedDownstreamAddr> shared_addr;
#ifdef HAVE_MRUBY
std::shared_ptr<mruby::MRubyContext> mruby_ctx;
#endif // HAVE_MRUBY
// true if this group is no longer used for new request. If this is // true if this group is no longer used for new request. If this is
// true, the connection made using one of address in shared_addr // true, the connection made using one of address in shared_addr
// must not be pooled. // must not be pooled.