diff --git a/include/dsn/tool-api/task.h b/include/dsn/tool-api/task.h index 2d576ebc5..4504f7fc0 100644 --- a/include/dsn/tool-api/task.h +++ b/include/dsn/tool-api/task.h @@ -307,6 +307,28 @@ class rpc_request_task : public task, public transient_object static_cast(_request->header->client.timeout_ms) * 1000000ULL) { _handler->run(_request); + // rpc_handler_info::run() consumes the dispatch reference. + _handler = nullptr; + } + else + { + release_handler(); + } + } + +private: + void release_handler() + { + rpc_handler_info* handler = _handler; + if (handler == nullptr) + { + return; + } + + _handler = nullptr; + if (1 == handler->release_ref()) + { + delete handler; } } diff --git a/src/core/src/env_provider.cpp b/src/core/src/env_provider.cpp index 55bbc445e..296bb5ed6 100644 --- a/src/core/src/env_provider.cpp +++ b/src/core/src/env_provider.cpp @@ -65,7 +65,17 @@ void env_provider::set_thread_local_random_seed(int s) uint64_t env_provider::random64(uint64_t min, uint64_t max) { - dassert(min <= max, "invalid random range"); + if (min > max) + { + // Callers may derive min/max from independent configuration values (e.g. the + // emulator's min/max message delay). An inverted range aborts here via the + // assert and, once asserts are compiled out, feeds an invalid range to + // uniform_int_distribution (undefined behavior). Degrade to a deterministic + // in-range value instead of crashing. + dwarn("invalid random range [%llu, %llu], returning %llu", + (unsigned long long)min, (unsigned long long)max, (unsigned long long)min); + return min; + } if (env_provider__tls_magic != 0xdeadbeef) { env_provider__rng = new std::remove_pointer::type(std::random_device{}()); diff --git a/src/core/src/group_address.h b/src/core/src/group_address.h index e74f1f223..a15229dac 100644 --- a/src/core/src/group_address.h +++ b/src/core/src/group_address.h @@ -131,7 +131,23 @@ namespace dsn inline rpc_address rpc_group_address::possible_leader() { - alr_t l(_lock); + { + // Fast path: a leader has already been chosen, so a shared read lock + // is enough here (we only read _members/_leader_index). + alr_t l(_lock); + if (_members.empty()) + return _invalid; + if (_leader_index != -1) + return _members[_leader_index]; + } + + // Slow path: no leader yet, so we must assign _leader_index, which requires + // the exclusive write lock (writing it under the shared read lock would be a + // data race; the sibling writers leader_forward()/set_leader() also take the + // write lock). Re-check under the write lock because another thread may have + // chosen the leader (or emptied _members) between releasing the read lock and + // acquiring the write lock. + alw_t l(_lock); if (_members.empty()) return _invalid; if (_leader_index == -1) diff --git a/src/core/src/main.cpp b/src/core/src/main.cpp index e79672cbf..cfcf16376 100644 --- a/src/core/src/main.cpp +++ b/src/core/src/main.cpp @@ -602,7 +602,11 @@ bool run( for (auto it = spec.toollets.begin(); it != spec.toollets.end(); ++it) { auto tlet = dsn::tools::internal_use_only::get_toollet(it->c_str(), ::dsn::PROVIDER_TYPE_MAIN); - dassert(tlet, "toolet not found"); + if (nullptr == tlet) + { + derror("toollet '%s' is not found, please check the [core] toollets configuration", it->c_str()); + return false; + } tlet->install(spec); } @@ -612,7 +616,12 @@ bool run( // TODO: register sys_exit execution // init runtime - ::dsn::service_engine::fast_instance().init_after_toollets(); + auto err = ::dsn::service_engine::fast_instance().init_after_toollets(); + if (err != ::dsn::ERR_OK) + { + derror("service engine init failed, err = %s", err.to_string()); + return false; + } dsn_all.engine_ready.store(true, std::memory_order_release); @@ -638,6 +647,14 @@ bool run( { ::dsn::safe_list< ::dsn::safe_string> argskvs; ::dsn::utils::split_args(kv.c_str(), argskvs, '@'); + // split_args drops empty/whitespace-only tokens, so a malformed app_list + // entry consisting only of '@' separators (e.g. -app_list "@") yields an + // empty list. front() on an empty std::list is undefined behavior, so skip + // such an entry: it names no app and cannot match any config section. + if (argskvs.empty()) + { + continue; + } if (::dsn::safe_string("apps.") + argskvs.front() == sp.config_section) { if (argskvs.size() < 2) diff --git a/src/core/src/network.cpp b/src/core/src/network.cpp index a74dfad94..c4869d4ca 100644 --- a/src/core/src/network.cpp +++ b/src/core/src/network.cpp @@ -579,7 +579,8 @@ namespace dsn char name[128]; if (gethostname(name, sizeof(name)) != 0) { - dassert(false, "gethostname failed, err = %s", strerror(errno)); + derror("gethostname failed, err = %s", strerror(errno)); + return 0; } ip = dsn_ipv4_from_host(name); } diff --git a/src/core/src/rpc.test.cpp b/src/core/src/rpc.test.cpp index acdd631f5..6a8a43661 100644 --- a/src/core/src/rpc.test.cpp +++ b/src/core/src/rpc.test.cpp @@ -37,6 +37,7 @@ #include #include #include "group_address.h" +#include "rpc_engine.h" #include #include #include @@ -45,6 +46,23 @@ typedef std::function rpc_reply_handler; +namespace { + +class expired_rpc_request_task : public ::dsn::rpc_request_task +{ +public: + expired_rpc_request_task(::dsn::message_ex* request, + ::dsn::rpc_handler_info* handler, + ::dsn::service_node* node) + : rpc_request_task(request, handler, node) + { + } + + void expire() { _enqueue_ts_ns = 1; } +}; + +} // anonymous namespace + static ::dsn::rpc_address build_group() { ::dsn::rpc_address server_group; server_group.assign_group(dsn_group_build("server_group.test")); @@ -93,6 +111,82 @@ TEST(core, rpc) EXPECT_TRUE(result.second == "server"); } +TEST(core, rpc_inline_dispatch_releases_handler_reference) +{ + ::dsn::rpc_server_dispatcher dispatcher; + ::dsn::rpc_handler_info handler(RPC_TEST_HASH); + int call_count = 0; + + handler.name = "rpc.inline.reference"; + handler.c_handler = [](dsn_message_t, void* context) { + ++*static_cast(context); + }; + handler.parameter = &call_count; + handler.add_ref(); + ASSERT_TRUE(dispatcher.register_rpc_handler(&handler)); + + auto request = ::dsn::message_ex::create_request(RPC_TEST_HASH, 100, 0, 0); + ASSERT_NE(nullptr, request); + request->add_ref(); + + dispatcher.on_request_with_inline_execution(request, nullptr); + + EXPECT_EQ(1, call_count); + EXPECT_EQ(1, handler.running_count.load(std::memory_order_relaxed)); + EXPECT_EQ(&handler, dispatcher.unregister_rpc_handler(RPC_TEST_HASH)); + + while (handler.running_count.load(std::memory_order_relaxed) > 0) + { + handler.release_ref(); + } + request->release_ref(); +} + +TEST(core, rpc_expired_dispatch_releases_handler_reference) +{ + ::dsn::rpc_handler_info handler(RPC_TEST_HASH); + int call_count = 0; + + handler.c_handler = [](dsn_message_t, void* context) { + ++*static_cast(context); + }; + handler.parameter = &call_count; + handler.add_ref(); + handler.add_ref(); + + auto request = ::dsn::message_ex::create_request(RPC_TEST_HASH, 0, 0, 0); + ASSERT_NE(nullptr, request); + { + expired_rpc_request_task task(request, &handler, nullptr); + task.expire(); + task.exec(); + } + + EXPECT_EQ(0, call_count); + EXPECT_EQ(1, handler.running_count.load(std::memory_order_relaxed)); + while (handler.running_count.load(std::memory_order_relaxed) > 0) + { + handler.release_ref(); + } +} + +TEST(core, rpc_discarded_dispatch_releases_handler_reference) +{ + ::dsn::rpc_handler_info handler(RPC_TEST_HASH); + handler.add_ref(); + handler.add_ref(); + + auto request = ::dsn::message_ex::create_request(RPC_TEST_HASH, 100, 0, 0); + ASSERT_NE(nullptr, request); + { ::dsn::rpc_request_task task(request, &handler, nullptr); } + + EXPECT_EQ(1, handler.running_count.load(std::memory_order_relaxed)); + while (handler.running_count.load(std::memory_order_relaxed) > 0) + { + handler.release_ref(); + } +} + TEST(core, group_address_talk_to_others) { ::dsn::rpc_address addr = build_group(); diff --git a/src/core/src/rpc_engine.cpp b/src/core/src/rpc_engine.cpp index fa32a0263..8ca91a699 100644 --- a/src/core/src/rpc_engine.cpp +++ b/src/core/src/rpc_engine.cpp @@ -455,7 +455,7 @@ namespace dsn { } else { - dassert(false, "rpc registration confliction for '%s'", name.c_str()); + derror("rpc registration confliction for '%s'", name.c_str()); return false; } } @@ -547,6 +547,10 @@ namespace dsn { if (handler) { handler->c_handler(msg, handler->parameter); + if (1 == handler->release_ref()) + { + delete handler; + } } else { @@ -636,13 +640,26 @@ namespace dsn { const service_spec& spec = service_engine::fast_instance().spec(); network* net = utils::factory_store::create( netcs.factory_name.c_str(), ::dsn::PROVIDER_TYPE_MAIN, this, nullptr); + if (net == nullptr) + { + derror("cannot create network provider '%s', please check the factory registration and configuration", + netcs.factory_name.c_str()); + return nullptr; + } net->reset_parser_attr(client_hdr_format, netcs.message_buffer_block_size); for (auto it = spec.network_aspects.begin(); it != spec.network_aspects.end(); it++) { - net = utils::factory_store::create(it->c_str(), ::dsn::PROVIDER_TYPE_ASPECT, this, net); + network* net2 = utils::factory_store::create(it->c_str(), ::dsn::PROVIDER_TYPE_ASPECT, this, net); + if (net2 == nullptr) + { + derror("cannot create network aspect provider '%s', please check the factory registration and configuration", + it->c_str()); + return nullptr; + } + net = net2; } // start the net @@ -939,7 +956,18 @@ namespace dsn { // still got time, retry uint64_t nms = dsn_now_ms(); - uint64_t gap = 8 << req2->send_retry_count; + // Bound the shift width: 8 << 7 (1024) already exceeds the + // 1000ms ceiling below, so clamping the exponent leaves the + // exponential backoff identical while avoiding the signed + // overflow / oversized-shift undefined behavior that occurs + // once send_retry_count grows large during long-timeout + // retries against a persistently failing partition. + int retry_shift = req2->send_retry_count; + if (retry_shift > 10) + { + retry_shift = 10; + } + uint64_t gap = 8ull << retry_shift; if (gap > 1000) gap = 1000; if (nms + gap < timeout_ts_ms) diff --git a/src/core/src/service_api_c.test.cpp b/src/core/src/service_api_c.test.cpp index 8d1429a38..210f9e9a8 100644 --- a/src/core/src/service_api_c.test.cpp +++ b/src/core/src/service_api_c.test.cpp @@ -526,6 +526,24 @@ TEST(core, dsn_rpc_registration_invalid_parameters) ASSERT_EQ(nullptr, dsn_rpc_unregiser_handler(TASK_CODE_INVALID, dsn_gpid())); } +TEST(core, dsn_rpc_registration_conflict) +{ + int first_context = 1; + int second_context = 2; + + ASSERT_TRUE(dsn_rpc_register_handler(TASK_CODE_RPC_FOR_TEST, + "first_handler", + noop_rpc_request_handler, + &first_context, + dsn_gpid())); + ASSERT_FALSE(dsn_rpc_register_handler(TASK_CODE_RPC_FOR_TEST, + "second_handler", + noop_rpc_request_handler, + &second_context, + dsn_gpid())); + ASSERT_EQ(&first_context, dsn_rpc_unregiser_handler(TASK_CODE_RPC_FOR_TEST, dsn_gpid())); +} + TEST(core, dsn_rpc_dispatch_invalid_parameters) { const dsn_address_t invalid_address = {}; diff --git a/src/core/src/service_engine.cpp b/src/core/src/service_engine.cpp index 2fde0960c..7511e2a59 100644 --- a/src/core/src/service_engine.cpp +++ b/src/core/src/service_engine.cpp @@ -201,6 +201,12 @@ error_code service_node::init_io_engine(io_engine& io, ioe_mode mode) aio = factory_store::create(it->c_str(), PROVIDER_TYPE_ASPECT, io.disk, aio); } + if (nullptr == aio) + { + derror("cannot create aio provider '%s', please check [core] aio_factory_name and aio_aspects", + spec.aio_factory_name.c_str()); + return ERR_SERVICE_NOT_FOUND; + } io.aio = aio; } else @@ -580,7 +586,7 @@ void service_engine::init_before_toollets(const service_spec& spec) ); } -void service_engine::init_after_toollets() +error_code service_engine::init_after_toollets() { // init common providers (second half) _env = factory_store::create(_spec.env_factory_name.c_str(), @@ -592,7 +598,14 @@ void service_engine::init_after_toollets() _env = factory_store::create(it->c_str(), PROVIDER_TYPE_ASPECT, _env); } + if (nullptr == _env) + { + derror("cannot create env provider '%s', please check [core] env_factory_name and env_aspects", + _spec.env_factory_name.c_str()); + return ERR_SERVICE_NOT_FOUND; + } tls_dsn.env = _env; + return ERR_OK; } void service_engine::register_system_rpc_handler( @@ -666,12 +679,17 @@ service_node* service_engine::start_node(service_app_spec& app_spec) { service_node* n = _nodes_by_app_port[p]; - dassert(false, "network port %d usage confliction for %s vs %s, " + // A port collision between two configured apps is an operator + // misconfiguration, not an internal invariant violation. Report it and + // fail node creation gracefully (callers below already handle a nullptr + // return) instead of aborting the whole process. + derror("network port %d usage confliction for %s vs %s, " "please reconfig", p, n->name(), app_spec.name.c_str() ); + return nullptr; } } diff --git a/src/core/src/service_engine.h b/src/core/src/service_engine.h index 72efdc6bc..dbcca3fda 100644 --- a/src/core/src/service_engine.h +++ b/src/core/src/service_engine.h @@ -155,7 +155,7 @@ class service_engine : public utils::singleton static safe_string get_queue_info(const safe_vector& args); void init_before_toollets(const service_spec& spec); - void init_after_toollets(); + error_code init_after_toollets(); void configuration_changed(); service_node* start_node(service_app_spec& app_spec); diff --git a/src/core/src/task.cpp b/src/core/src/task.cpp index 956950c91..a5ae77988 100644 --- a/src/core/src/task.cpp +++ b/src/core/src/task.cpp @@ -591,6 +591,7 @@ rpc_request_task::rpc_request_task(message_ex* request, rpc_handler_info* h, ser rpc_request_task::~rpc_request_task() { + release_handler(); _request->release_ref(); // added in ctor } diff --git a/src/core/src/task_engine.cpp b/src/core/src/task_engine.cpp index 19e69b2e4..0d3561a9e 100644 --- a/src/core/src/task_engine.cpp +++ b/src/core/src/task_engine.cpp @@ -69,6 +69,13 @@ void task_worker_pool::create() { q = factory_store::create(it->c_str(), PROVIDER_TYPE_ASPECT, this, i, q); } + if (nullptr == q) + { + derror("cannot create task queue (or a queue aspect) for thread pool '%s', " + "please check the [threadpool.%s] queue_factory_name / queue_aspects configuration", + _spec.name.c_str(), _spec.name.c_str()); + dsn_exit(1); + } _queues.push_back(q); if (_spec.admission_controller_factory_name != "") @@ -103,6 +110,13 @@ void task_worker_pool::create() { worker = factory_store::create(it->c_str(), PROVIDER_TYPE_ASPECT, this, q, i, worker); } + if (nullptr == worker) + { + derror("cannot create task worker (or a worker aspect) for thread pool '%s', " + "please check the [threadpool.%s] worker_factory_name / worker_aspects configuration", + _spec.name.c_str(), _spec.name.c_str()); + dsn_exit(1); + } task_worker::on_create.execute(worker); q->set_owner_worker(spec().partitioned ? worker : nullptr); diff --git a/src/core/src/task_spec.cpp b/src/core/src/task_spec.cpp index 0d2b7156c..10d9b1068 100644 --- a/src/core/src/task_spec.cpp +++ b/src/core/src/task_spec.cpp @@ -223,9 +223,15 @@ bool task_spec::init() if (!read_config(section_name.c_str(), *spec, &default_spec)) return false; - dassert(spec->rpc_request_delays_milliseconds.size() == 0 - || spec->rpc_request_delays_milliseconds.size() == 6, - "invalid length of rpc_request_delays_milliseconds, must be of length 6"); + if (spec->rpc_request_delays_milliseconds.size() != 0 + && spec->rpc_request_delays_milliseconds.size() != 6) + { + derror("%s: invalid length (%d) of rpc_request_delays_milliseconds, " + "must be 0 or 6", + spec->name.c_str(), + (int)spec->rpc_request_delays_milliseconds.size()); + return false; + } if (spec->rpc_request_delays_milliseconds.size() > 0) { std::vector mss{ spec->rpc_request_delays_milliseconds.begin(), @@ -324,6 +330,18 @@ bool threadpool_spec::init(/*out*/ safe_vector& specs) if ("" == spec.name) spec.name = dsn_threadpool_code_to_string(code); + if (spec.worker_count < 1) + { + // A partitioned pool with worker_count == 0 creates empty worker/queue + // vectors, so task_worker_pool::enqueue() and shared_same_worker_with_current_task() + // divide by _queues.size()/_workers.size() == 0 (SIGFPE); a non-partitioned pool + // with 0 workers silently hangs (tasks enqueued but never processed). Every pool + // needs at least one worker, so reject the misconfiguration up front. + derror("invalid worker_count %d for thread pool %s (%s); worker_count must be >= 1", + spec.worker_count, spec.name.c_str(), dsn_threadpool_code_to_string(code)); + return false; + } + if (false == spec.worker_share_core && 0 == spec.worker_affinity_mask) { // worker_affinity_mask is a 64-bit mask. Shifting the literal int 1 by diff --git a/src/dev/cpp/file_utils.cpp b/src/dev/cpp/file_utils.cpp index bdc78fff0..0b7659431 100644 --- a/src/dev/cpp/file_utils.cpp +++ b/src/dev/cpp/file_utils.cpp @@ -1070,13 +1070,23 @@ namespace dsn { } } - if (::QueryFullProcessImageNameA( + BOOL query_ok = ::QueryFullProcessImageNameA( hProcess, 0, tls_path_buffer, &dwSize - ) == FALSE - ) + ); + + // OpenProcess() (pid != -1) returns a real handle that must be closed on every + // exit path; GetCurrentProcess() (pid == -1) returns a pseudo-handle that must + // not be closed. Previously the handle was leaked when QueryFullProcessImageNameA + // failed. + if (pid != -1) + { + ::CloseHandle(hProcess); + } + + if (query_ok == FALSE) { return ERR_PATH_NOT_FOUND; } @@ -1096,6 +1106,15 @@ namespace dsn { return ERR_PATH_NOT_FOUND; } + // readlink() does not null-terminate and returns up to TLS_PATH_BUFFER_SIZE + // bytes. When the target path length >= TLS_PATH_BUFFER_SIZE it returns exactly + // TLS_PATH_BUFFER_SIZE, so writing the NUL at tls_path_buffer[err] would be one + // byte past the end of the buffer. Clamp to the last valid index (the path is + // truncated in that case), mirroring the FreeBSD/Apple branches below. + if (err >= TLS_PATH_BUFFER_SIZE) + { + err = TLS_PATH_BUFFER_SIZE - 1; + } tls_path_buffer[err] = 0; path = tls_path_buffer; # elif defined(__FreeBSD__) diff --git a/src/dev/cpp/perf_test_helper.cpp b/src/dev/cpp/perf_test_helper.cpp index ce6219d22..11baff0bf 100644 --- a/src/dev/cpp/perf_test_helper.cpp +++ b/src/dev/cpp/perf_test_helper.cpp @@ -55,7 +55,12 @@ namespace dsn { if (!read_config("task..default", _default_opts)) { - dassert(false, "read configuration failed for section [task..default]"); + // A malformed value in [task..default] (e.g. a non-integer in one of + // the INT_LIST fields) makes read_config return false. Report it and + // continue with defaults instead of aborting the whole process; the + // empty-list checks below fill in sensible defaults for any field that + // was not parsed. + derror("read configuration failed for section [task..default], using defaults"); } if (_default_opts.perf_test_concurrency.size() == 0) @@ -110,7 +115,13 @@ namespace dsn { perf_test_opts opt; if (!read_config(s.config_section, opt, &_default_opts)) { - dassert(false, "read configuration failed for section [%s]", s.config_section); + // A malformed value in this perf-test section (e.g. a non-integer in one + // of the INT_LIST fields) makes read_config return false. Report it and + // skip this suite instead of aborting the whole benchmark process; the + // suite is left with no cases (its cases were cleared by the caller). + derror("read configuration failed for section [%s], skipping this suite", + s.config_section); + return; } double ratio_sum = 0.0; @@ -135,6 +146,13 @@ namespace dsn { c.seconds = opt.perf_test_seconds; c.payload_bytes = bytes; c.key_space_size = opt.perf_test_key_space_size; + if (c.key_space_size < 1) + { + // Perf clients derive keys with "random % key_space_size"; a + // configured value of 0 (or negative) would divide by zero + // (SIGFPE). Clamp centrally so every client is protected. + c.key_space_size = 1; + } c.timeout_ms = opt.perf_test_timeouts_ms[i]; c.concurrency = cc; c.ratios.resize(max_request_kind_count_for_hybrid_test, 0.0); diff --git a/src/plugins/apps.skv/simple_kv.server.impl.cpp b/src/plugins/apps.skv/simple_kv.server.impl.cpp index fa225213d..10b170bc6 100644 --- a/src/plugins/apps.skv/simple_kv.server.impl.cpp +++ b/src/plugins/apps.skv/simple_kv.server.impl.cpp @@ -191,7 +191,10 @@ namespace dsn { if (!dsn::utils::filesystem::remove_path(data_dir())) { - dassert(false, "Fail to delete directory %s.", data_dir()); + // A cleanup failure (directory busy, permission, transient FS + // error) must not abort the process during shutdown; log and + // continue so stop() completes. + derror("Fail to delete directory %s.", data_dir()); } } } @@ -448,7 +451,27 @@ namespace dsn { else { dassert(DSN_CHKPT_COPY == mode, "invalid mode %d", (int)mode); - dassert(state.to_decree_included > last_durable_decree(), "checkpoint's decree is smaller than current"); + + // The COPY branch dereferences state.files[0] below. The sibling LEARN + // branch above guards file_state_count <= 0 before touching files[0]; + // do the same here so an empty/malformed learn state is rejected instead + // of indexing a null/absent file entry (out-of-bounds / null deref). + if (state.file_state_count <= 0) + { + derror("simple_kv_service_impl copy checkpoint failed: no checkpoint files provided"); + return ERR_CHECKPOINT_FAILED; + } + + // A checkpoint whose decree is not newer than the current durable state + // is a recoverable protocol/ordering condition, not an internal invariant. + // Reject it gracefully rather than aborting the whole process. + if (state.to_decree_included <= last_durable_decree()) + { + derror("simple_kv_service_impl copy checkpoint failed: checkpoint decree %" PRId64 + " is not greater than current durable decree %" PRId64, + state.to_decree_included, last_durable_decree()); + return ERR_CHECKPOINT_FAILED; + } char name[256]; int len = snprintf(name, sizeof(name), "%s/checkpoint.%" PRId64, diff --git a/src/plugins/tools.common/asio_net_provider.cpp b/src/plugins/tools.common/asio_net_provider.cpp index b0669586b..0887ad83e 100644 --- a/src/plugins/tools.common/asio_net_provider.cpp +++ b/src/plugins/tools.common/asio_net_provider.cpp @@ -95,7 +95,11 @@ namespace dsn { _acceptor = nullptr; - dassert(channel == RPC_CHANNEL_TCP || channel == RPC_CHANNEL_UDP, "invalid given channel %s", channel.to_string()); + if (channel != RPC_CHANNEL_TCP && channel != RPC_CHANNEL_UDP) + { + derror("invalid given channel %s", channel.to_string()); + return ERR_NOT_IMPLEMENTED; + } _address.assign_ipv4(get_local_ipv4(), port); @@ -330,7 +334,11 @@ namespace dsn { int io_service_worker_count = (int)dsn_config_get_value_uint64("network", "io_service_worker_count", 1, "thread number for io service (timer and boost network)"); - dassert(channel == RPC_CHANNEL_UDP, "invalid given channel %s", channel.to_string()); + if (channel != RPC_CHANNEL_UDP) + { + derror("invalid given channel %s", channel.to_string()); + return ERR_NOT_IMPLEMENTED; + } if (client_only) { diff --git a/src/plugins/tools.common/simple_logger.cpp b/src/plugins/tools.common/simple_logger.cpp index c56fb1357..676702c38 100644 --- a/src/plugins/tools.common/simple_logger.cpp +++ b/src/plugins/tools.common/simple_logger.cpp @@ -103,8 +103,16 @@ namespace dsn { "copy log messages at or above this level to stderr in addition to logfiles"), LOG_LEVEL_INVALID ); - dassert(_stderr_start_level != LOG_LEVEL_INVALID, - "invalid [tools.simple_logger] stderr_start_level specified"); + if (_stderr_start_level == LOG_LEVEL_INVALID) + { + // Do not abort logger initialization on an invalid config value: fall back to the + // default level. Use stderr directly because the logging system is not fully + // initialized at this point. + fprintf(stderr, + "invalid [tools.simple_logger] stderr_start_level specified, " + "falling back to WARNING\n"); + _stderr_start_level = LOG_LEVEL_WARNING; + } _max_number_of_log_files_on_disk = dsn_config_get_value_uint64( "tools.simple_logger", diff --git a/src/plugins/tools.common/simple_perf_counter_v2_atomic.cpp b/src/plugins/tools.common/simple_perf_counter_v2_atomic.cpp index 8ba151f44..27d14d253 100644 --- a/src/plugins/tools.common/simple_perf_counter_v2_atomic.cpp +++ b/src/plugins/tools.common/simple_perf_counter_v2_atomic.cpp @@ -213,6 +213,13 @@ namespace dsn { "counter_computation_interval_seconds", 30, "period (seconds) the system computes the percentiles of the counters"); + if (_counter_computation_interval_seconds < 1) + { + // A configured value of 0 (or one that truncates to 0 through the (int) + // cast) makes "rand() % _counter_computation_interval_seconds" a division + // by zero (SIGFPE). Match the guard the base simple_perf_counter uses. + _counter_computation_interval_seconds = 1; + } _timer.reset(new boost::asio::deadline_timer(shared_io_service::instance().ios)); _timer->expires_from_now(boost::posix_time::seconds(rand() % _counter_computation_interval_seconds + 1)); this->add_ref(); diff --git a/src/plugins/tools.common/simple_perf_counter_v2_fast.cpp b/src/plugins/tools.common/simple_perf_counter_v2_fast.cpp index ac11d7a4a..b1a664d4a 100644 --- a/src/plugins/tools.common/simple_perf_counter_v2_fast.cpp +++ b/src/plugins/tools.common/simple_perf_counter_v2_fast.cpp @@ -211,6 +211,13 @@ namespace dsn { "counter_computation_interval_seconds", 30, "period (seconds) the system computes the percentiles of the counters"); + if (_counter_computation_interval_seconds < 1) + { + // A configured value of 0 (or one that truncates to 0 through the (int) + // cast) makes "rand() % _counter_computation_interval_seconds" a division + // by zero (SIGFPE). Match the guard the base simple_perf_counter uses. + _counter_computation_interval_seconds = 1; + } _timer.reset(new boost::asio::deadline_timer(shared_io_service::instance().ios)); _timer->expires_from_now(boost::posix_time::seconds(rand() % _counter_computation_interval_seconds + 1)); this->add_ref(); diff --git a/src/plugins/tools.emulator/network.sim.cpp b/src/plugins/tools.emulator/network.sim.cpp index 4d4faba59..702c19a45 100644 --- a/src/plugins/tools.emulator/network.sim.cpp +++ b/src/plugins/tools.emulator/network.sim.cpp @@ -190,11 +190,25 @@ namespace dsn { namespace tools { error_code sim_network_provider::start(rpc_channel channel, int port, bool client_only, io_modifer& ctx) { - dassert(channel == RPC_CHANNEL_TCP || channel == RPC_CHANNEL_UDP, "invalid given channel %s", channel.to_string()); + if (channel != RPC_CHANNEL_TCP && channel != RPC_CHANNEL_UDP) + { + // channel is a customizable_id that can be set from configuration; an + // unsupported value is an operator error, not an internal invariant. Reject + // it gracefully instead of aborting. + derror("invalid channel %s for emulator network, only TCP and UDP are supported", + channel.to_string()); + return ERR_NOT_IMPLEMENTED; + } _address = ::dsn::rpc_address("localhost", port); auto hostname = ::dsn::utils::asio::host_name(); - dassert(!hostname.empty(), "fail to get local hostname"); + if (hostname.empty()) + { + // Resolving the local hostname can fail on a misconfigured host; fail network + // startup gracefully rather than aborting the process. + derror("fail to get local hostname"); + return ERR_NETWORK_INIT_FAILED; + } if (!client_only) { for (int i = NET_HDR_INVALID + 1; i <= network_header_format::max_value(); i++) diff --git a/src/plugins_ext/rDSN.dist.deployment b/src/plugins_ext/rDSN.dist.deployment index e1af16d7d..7a9dc7345 160000 --- a/src/plugins_ext/rDSN.dist.deployment +++ b/src/plugins_ext/rDSN.dist.deployment @@ -1 +1 @@ -Subproject commit e1af16d7db6ad48ac98a314ebfe049c690702713 +Subproject commit 7a9dc7345fd104f5adccbe7ba2f2a97931ddd4a8 diff --git a/src/plugins_ext/rDSN.dist.service b/src/plugins_ext/rDSN.dist.service index 7300df581..b34bb9020 160000 --- a/src/plugins_ext/rDSN.dist.service +++ b/src/plugins_ext/rDSN.dist.service @@ -1 +1 @@ -Subproject commit 7300df5815add609c2a6e5d25943d4b3388d545d +Subproject commit b34bb902019ac852ae8eaba9ae4ba313c8ada3cc diff --git a/src/plugins_ext/rDSN.tools.hpc b/src/plugins_ext/rDSN.tools.hpc index 1089edfcf..98d869962 160000 --- a/src/plugins_ext/rDSN.tools.hpc +++ b/src/plugins_ext/rDSN.tools.hpc @@ -1 +1 @@ -Subproject commit 1089edfcff5201d179139e31ecedafb102ee73e5 +Subproject commit 98d869962632e724e176c0f0c12069f16e3d308d diff --git a/src/plugins_ext/rDSN.tools.log.monitor b/src/plugins_ext/rDSN.tools.log.monitor index ef3cbf857..e92b56bc0 160000 --- a/src/plugins_ext/rDSN.tools.log.monitor +++ b/src/plugins_ext/rDSN.tools.log.monitor @@ -1 +1 @@ -Subproject commit ef3cbf857558b61ab3d5bbb0b82a9da99e2bf95a +Subproject commit e92b56bc0325db66f13f67a25f42863cbe729c9a