From dc091fc952f953f3f6efee06ea3440ef4e5ef1ad Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Tue, 19 Oct 2021 10:57:09 +0300 Subject: [PATCH] effective_replication_map, abstract_replication_strategy: get_ranges: call on_internal_error in empty sorted_tokens case Accessing tm.sorted_tokens().back() causes undefined behavior if tm.sorted_tokens is empty. Check that first and throw/abort using on_internal_error in this case. This will prevent the segfault but it doesn't fix the root cause which is getting here with empty token_metadata. That will be fixed by the following patch. Refs #9494 Signed-off-by: Benny Halevy Message-Id: <20211019075710.1626808-1-bhalevy@scylladb.com> --- locator/abstract_replication_strategy.cc | 16 ++++++++++++---- locator/abstract_replication_strategy.hh | 7 +++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/locator/abstract_replication_strategy.cc b/locator/abstract_replication_strategy.cc index c314aa7b45..a29b2c9c68 100644 --- a/locator/abstract_replication_strategy.cc +++ b/locator/abstract_replication_strategy.cc @@ -156,8 +156,12 @@ dht::token_range_vector effective_replication_map::do_get_ranges(noncopyable_function should_add_range) const { dht::token_range_vector ret; const auto& tm = *_tmptr; - auto prev_tok = tm.sorted_tokens().back(); - for (const auto& tok : tm.sorted_tokens()) { + const auto& sorted_tokens = tm.sorted_tokens(); + if (sorted_tokens.empty()) { + on_internal_error(rslogger, "Token metadata is empty"); + } + auto prev_tok = sorted_tokens.back(); + for (const auto& tok : sorted_tokens) { if (should_add_range(get_natural_endpoints(tok))) { insert_token_range_to_sorted_container_while_unwrapping(prev_tok, tok, ret); } @@ -183,8 +187,12 @@ future abstract_replication_strategy::get_ranges(inet_address ep, token_metadata_ptr tmptr) const { dht::token_range_vector ret; const auto& tm = *tmptr; - auto prev_tok = tm.sorted_tokens().back(); - for (auto tok : tm.sorted_tokens()) { + const auto& sorted_tokens = tm.sorted_tokens(); + if (sorted_tokens.empty()) { + on_internal_error(rslogger, "Token metadata is empty"); + } + auto prev_tok = sorted_tokens.back(); + for (auto tok : sorted_tokens) { for (inet_address a : co_await calculate_natural_endpoints(tok, tm)) { if (a == ep) { insert_token_range_to_sorted_container_while_unwrapping(prev_tok, tok, ret); diff --git a/locator/abstract_replication_strategy.hh b/locator/abstract_replication_strategy.hh index 117d148df4..9eac93a793 100644 --- a/locator/abstract_replication_strategy.hh +++ b/locator/abstract_replication_strategy.hh @@ -117,6 +117,7 @@ public: replication_strategy_type get_type() const { return _my_type; } // Use the token_metadata provided by the caller instead of _token_metadata + // Note: must be called with initialized, non-empty token_metadata. future get_ranges(inet_address ep, token_metadata_ptr tmptr) const; public: @@ -175,6 +176,8 @@ public: // The list is sorted, and its elements are non overlapping and non wrap-around. // It the analogue of Origin's getAddressRanges().get(endpoint). // This function is not efficient, and not meant for the fast path. + // + // Note: must be called after token_metadata has been initialized. dht::token_range_vector get_ranges(inet_address ep) const; // get_primary_ranges() returns the list of "primary ranges" for the given @@ -183,11 +186,15 @@ public: // returned calculate_natural_endpoints(). // This function is the analogue of Origin's // StorageService.getPrimaryRangesForEndpoint(). + // + // Note: must be called after token_metadata has been initialized. dht::token_range_vector get_primary_ranges(inet_address ep) const; // get_primary_ranges_within_dc() is similar to get_primary_ranges() // except it assigns a primary node for each range within each dc, // instead of one node globally. + // + // Note: must be called after token_metadata has been initialized. dht::token_range_vector get_primary_ranges_within_dc(inet_address ep) const; std::unordered_map