Skip to content

Commit

Permalink
distributed_loader::do_populate_column_family: do_with verifier
Browse files Browse the repository at this point in the history 
Rather than using lw_shared_ptr to keep it around, use do_with.
Fixes a use-after-free seen with interrupt_build_process_with_resharding_half_to_max_test
in debug mode:

==406079==ERROR: AddressSanitizer: heap-use-after-free on address 0x603001320ac0 at pc 0x00000777a062 bp 0x7ffdeb77b010 sp 0x7ffdeb77b000
READ of size 8 at 0x603001320ac0 thread T0
    #0 0x777a061 in _M_next /usr/include/c++/9/bits/hashtable_policy.h:283
    scylladb#1 0x779ead7 in _M_incr /usr/include/c++/9/bits/hashtable_policy.h:299
    scylladb#2 0x777bbac in operator++ /usr/include/c++/9/bits/hashtable_policy.h:362
    scylladb#3 0x77f8384 in run_and_dispose /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/future-util.hh:568
    scylladb#4 0x9808883 in seastar::reactor::run_tasks(seastar::reactor::task_queue&) /local/home/bhalevy/dev/scylla/seastar/src/core/reactor.cc:2091
    scylladb#5 0x980c831 in seastar::reactor::run_some_tasks() /local/home/bhalevy/dev/scylla/seastar/src/core/reactor.cc:2506
    scylladb#6 0x9811c44 in seastar::reactor::run() /local/home/bhalevy/dev/scylla/seastar/src/core/reactor.cc:2666
    scylladb#7 0x967db93 in seastar::app_template::run_deprecated(int, char**, std::function<void ()>&&) /local/home/bhalevy/dev/scylla/seastar/src/core/app-template.cc:199
    scylladb#8 0x967bbb9 in seastar::app_template::run(int, char**, std::function<seastar::future<int> ()>&&) /local/home/bhalevy/dev/scylla/seastar/src/core/app-template.cc:115
    scylladb#9 0x475fcb in main /local/home/bhalevy/dev/scylla/main.cc:486
    scylladb#10 0x7fb862d981a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)
    scylladb#11 0x42b69d in _start (/local/home/bhalevy/.dtest/dtest-q85idok7/test/node1/bin/scylla+0x42b69d)

0x603001320ac0 is located 0 bytes inside of 32-byte region [0x603001320ac0,0x603001320ae0)
freed by thread T0 here:
    #0 0x7fb86520996f in operator delete(void*) (/lib64/libasan.so.5+0x11096f)
    scylladb#1 0x77d1889 in deallocate /usr/include/c++/9/ext/new_allocator.h:128
    scylladb#2 0x77bcbb2 in deallocate /usr/include/c++/9/bits/alloc_traits.h:470
    scylladb#3 0x77bccc5 in _M_deallocate_node_ptr /usr/include/c++/9/bits/hashtable_policy.h:2113
    scylladb#4 0x779dc59 in _M_deallocate_node /usr/include/c++/9/bits/hashtable_policy.h:2103
    scylladb#5 0x77bc8b6 in _M_deallocate_nodes /usr/include/c++/9/bits/hashtable_policy.h:2124
    scylladb#6 0x779d18f in clear /usr/include/c++/9/bits/hashtable.h:2028
    scylladb#7 0x7779b8e in ~_Hashtable /usr/include/c++/9/bits/hashtable.h:1352
    scylladb#8 0x774cfed in ~unordered_map /usr/include/c++/9/bits/unordered_map.h:102
    scylladb#9 0x774d288 in ~shared_ptr_no_esft /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/shared_ptr.hh:157
    scylladb#10 0x774d2f1 in dispose /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/shared_ptr.hh:213
    scylladb#11 0x76f9642 in ~lw_shared_ptr /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/shared_ptr.hh:300
    scylladb#12 0x76f9642 in ~<lambda> /local/home/bhalevy/dev/scylla/distributed_loader.cc:743
    scylladb#13 0x771e0a7 in ~<lambda> /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/future.hh:1167
    scylladb#14 0x777c64c in destroy /local/home/bhalevy/dev/scylla/seastar/include/seastar/util/noncopyable_function.hh:133
    scylladb#15 0x586059 in seastar::noncopyable_function<seastar::future<> ()>::~noncopyable_function() /local/home/bhalevy/dev/scylla/seastar/include/seastar/util/noncopyable_function.hh:165
    scylladb#16 0x5867da in seastar::future<>::then_impl<seastar::noncopyable_function<seastar::future<> ()>, seastar::future<> >(seastar::noncopyable_function<seastar::future<> ()>&&)::{lambda()scylladb#1}::operator()() const::{lambda(seastar::future_state<>&&)scylladb#1}::~future_state() /local/home/bha
levy/dev/scylla/seastar/include/seastar/core/future.hh:1192
    scylladb#17 0xab704d in seastar::continuation<seastar::future<>::then_impl<seastar::noncopyable_function<seastar::future<> ()>, seastar::future<> >(seastar::noncopyable_function<seastar::future<> ()>&&)::{lambda()scylladb#1}::operator()() const::{lambda(seastar::future_state<>&&)scylladb#1}>::~continu
ation() /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/future.hh:507
    scylladb#18 0xab7216 in seastar::continuation<seastar::future<>::then_impl<seastar::noncopyable_function<seastar::future<> ()>, seastar::future<> >(seastar::noncopyable_function<seastar::future<> ()>&&)::{lambda()scylladb#1}::operator()() const::{lambda(seastar::future_state<>&&)scylladb#1}>::run_and_
dispose() /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/future.hh:512
    scylladb#19 0x9808883 in seastar::reactor::run_tasks(seastar::reactor::task_queue&) /local/home/bhalevy/dev/scylla/seastar/src/core/reactor.cc:2091
    scylladb#20 0x980c831 in seastar::reactor::run_some_tasks() /local/home/bhalevy/dev/scylla/seastar/src/core/reactor.cc:2506
    scylladb#21 0x9811c44 in seastar::reactor::run() /local/home/bhalevy/dev/scylla/seastar/src/core/reactor.cc:2666
    scylladb#22 0x967db93 in seastar::app_template::run_deprecated(int, char**, std::function<void ()>&&) /local/home/bhalevy/dev/scylla/seastar/src/core/app-template.cc:199
    scylladb#23 0x967bbb9 in seastar::app_template::run(int, char**, std::function<seastar::future<int> ()>&&) /local/home/bhalevy/dev/scylla/seastar/src/core/app-template.cc:115
    scylladb#24 0x475fcb in main /local/home/bhalevy/dev/scylla/main.cc:486
    scylladb#25 0x7fb862d981a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

previously allocated by thread T0 here:
    #0 0x7fb8652089d7 in operator new(unsigned long) (/lib64/libasan.so.5+0x10f9d7)
    scylladb#1 0x77d1796 in allocate /usr/include/c++/9/ext/new_allocator.h:114
    scylladb#2 0x77bcab3 in allocate /usr/include/c++/9/bits/alloc_traits.h:444
    scylladb#3 0x779d936 in _M_allocate_node<long int&, distributed_loader::cleanup_column_family_sstables(seastar::distributed<database>&, seastar::sstring, seastar::sstring, seastar::sstring)::sstable_descriptor> /usr/include/c++/9/bits/hashtable_policy.h:2081
    scylladb#4 0x777a2ed in _M_emplace<long int&, distributed_loader::cleanup_column_family_sstables(seastar::distributed<database>&, seastar::sstring, seastar::sstring, seastar::sstring)::sstable_descriptor> /usr/include/c++/9/bits/hashtable.h:1638
    scylladb#5 0x774d707 in emplace<long int&, distributed_loader::cleanup_column_family_sstables(seastar::distributed<database>&, seastar::sstring, seastar::sstring, seastar::sstring)::sstable_descriptor> /usr/include/c++/9/bits/hashtable.h:749
    scylladb#6 0x771ce3e in emplace<long int&, distributed_loader::cleanup_column_family_sstables(seastar::distributed<database>&, seastar::sstring, seastar::sstring, seastar::sstring)::sstable_descriptor> /usr/include/c++/9/bits/unordered_map.h:388
    scylladb#7 0x76f8280 in operator() /local/home/bhalevy/dev/scylla/distributed_loader.cc:732
    scylladb#8 0x774dc5d in _M_invoke /usr/include/c++/9/bits/std_function.h:286
    scylladb#9 0x6cde67b in std::function<seastar::future<> (std::filesystem::__cxx11::path, seastar::directory_entry)>::operator()(std::filesystem::__cxx11::path, seastar::directory_entry) const /usr/include/c++/9/bits/std_function.h:690
    scylladb#10 0x6cd345d in operator() /local/home/bhalevy/dev/scylla/lister.cc:30
    scylladb#11 0x6cd96e0 in apply /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/apply.hh:36
    scylladb#12 0x6cd9854 in apply<lister::visit(seastar::directory_entry)::<lambda(seastar::directory_entry)>&, seastar::directory_entry&&> /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/apply.hh:44
    scylladb#13 0x6cd9953 in apply<lister::visit(seastar::directory_entry)::<lambda(seastar::directory_entry)>&, seastar::directory_entry&&> /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/future.hh:1650
    scylladb#14 0x6cd6ba9 in operator() /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/future.hh:1168
    scylladb#15 0x6cdbac0 in call /local/home/bhalevy/dev/scylla/seastar/include/seastar/util/noncopyable_function.hh:101
    scylladb#16 0x6ce6178 in seastar::noncopyable_function<seastar::future<> (seastar::directory_entry&&)>::operator()(seastar::directory_entry&&) const /local/home/bhalevy/dev/scylla/seastar/include/seastar/util/noncopyable_function.hh:184
    scylladb#17 0x6ce3033 in seastar::apply_helper<seastar::noncopyable_function<seastar::future<> (seastar::directory_entry&&)>, std::tuple<seastar::directory_entry>&&, std::integer_sequence<unsigned long, 0ul> >::apply(seastar::noncopyable_function<seastar::future<> (seastar::directory_e
ntry&&)>&&, std::tuple<seastar::directory_entry>&&) /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/apply.hh:36
    scylladb#18 0x6ce3130 in auto seastar::apply<seastar::noncopyable_function<seastar::future<> (seastar::directory_entry&&)>, seastar::directory_entry>(seastar::noncopyable_function<seastar::future<> (seastar::directory_entry&&)>&&, std::tuple<seastar::directory_entry>&&) /local/home/bha
levy/dev/scylla/seastar/include/seastar/core/apply.hh:44
    scylladb#19 0x6ce322f in seastar::future<> seastar::futurize<seastar::future<> >::apply<seastar::noncopyable_function<seastar::future<> (seastar::directory_entry&&)>, seastar::directory_entry>(seastar::noncopyable_function<seastar::future<> (seastar::directory_entry&&)>&&, std::tuple<s
eastar::directory_entry>&&) /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/future.hh:1650
    scylladb#20 0x6cdf88d in seastar::future<seastar::directory_entry>::then_impl<seastar::noncopyable_function<seastar::future<> (seastar::directory_entry&&)>, seastar::future<> >(seastar::noncopyable_function<seastar::future<> (seastar::directory_entry&&)>&&)::{lambda()scylladb#1}::operator()()
const::{lambda(seastar::future_state<seastar::directory_entry>&&)scylladb#1}::operator()(seastar::future_state<seastar::directory_entry>) /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/future.hh:1196
    scylladb#21 0x6cea2e0 in seastar::continuation<seastar::future<seastar::directory_entry>::then_impl<seastar::noncopyable_function<seastar::future<> (seastar::directory_entry&&)>, seastar::future<> >(seastar::noncopyable_function<seastar::future<> (seastar::directory_entry&&)>&&)::{lamb
da()scylladb#1}::operator()() const::{lambda(seastar::future_state<seastar::directory_entry>&&)scylladb#1}, seastar::directory_entry>::run_and_dispose() /local/home/bhalevy/dev/scylla/seastar/include/seastar/core/future.hh:511
    scylladb#22 0x9808883 in seastar::reactor::run_tasks(seastar::reactor::task_queue&) /local/home/bhalevy/dev/scylla/seastar/src/core/reactor.cc:2091
    scylladb#23 0x980c831 in seastar::reactor::run_some_tasks() /local/home/bhalevy/dev/scylla/seastar/src/core/reactor.cc:2506
    scylladb#24 0x9811c44 in seastar::reactor::run() /local/home/bhalevy/dev/scylla/seastar/src/core/reactor.cc:2666
    scylladb#25 0x967db93 in seastar::app_template::run_deprecated(int, char**, std::function<void ()>&&) /local/home/bhalevy/dev/scylla/seastar/src/core/app-template.cc:199
    scylladb#26 0x967bbb9 in seastar::app_template::run(int, char**, std::function<seastar::future<int> ()>&&) /local/home/bhalevy/dev/scylla/seastar/src/core/app-template.cc:115
    scylladb#27 0x475fcb in main /local/home/bhalevy/dev/scylla/main.cc:486
    scylladb#28 0x7fb862d981a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

SUMMARY: AddressSanitizer: heap-use-after-free /usr/include/c++/9/bits/hashtable_policy.h:283 in _M_next
Shadow bytes around the buggy address:
  0x0c068025c100: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa
  0x0c068025c110: fd fd fd fa fa fa fd fd fd fa fa fa fd fd fd fa
  0x0c068025c120: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd
  0x0c068025c130: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa
  0x0c068025c140: fd fd fd fa fa fa 00 00 00 fa fa fa fd fd fd fa
=>0x0c068025c150: fa fa fd fd fd fa fa fa[fd]fd fd fd fa fa fd fd
  0x0c068025c160: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa
  0x0c068025c170: fd fd fd fa fa fa fd fd fd fa fa fa fd fd fd fa
  0x0c068025c180: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd
  0x0c068025c190: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa
  0x0c068025c1a0: fd fd fd fd fa fa fd fd fd fd fa fa 00 00 00 fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==406079==ABORTING

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
  • Loading branch information
@bhalevy
bhalevy committed Apr 17, 2020
1 parent 7520cf1 commit 2d255d0
Showing 1 changed file with 13 additions and 14 deletions.
27 changes: 13 additions & 14 deletions distributed_loader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -679,41 +679,40 @@ future<> distributed_loader::do_populate_column_family(distributed<database>& db
sstables::sstable::format_types format;
};

auto verifier = make_lw_shared<std::unordered_map<unsigned long, sstable_descriptor>>();

return do_with(std::vector<future<>>(), [&db, sstdir = std::move(sstdir), verifier, ks, cf] (std::vector<future<>>& futures) {
return lister::scan_dir(sstdir, { directory_entry_type::regular }, [&db, verifier, &futures] (fs::path sstdir, directory_entry de) {
return do_with(std::vector<future<>>(), std::unordered_map<unsigned long, sstable_descriptor>(),
[&db, sstdir = std::move(sstdir), ks, cf] (std::vector<future<>>& futures, auto& verifier) {
return lister::scan_dir(sstdir, { directory_entry_type::regular }, [&db, &verifier, &futures] (fs::path sstdir, directory_entry de) {
// FIXME: The secondary indexes are in this level, but with a directory type, (starting with ".")

// push future returned by probe_file into an array of futures,
// so that the supplied callback will not block scan_dir() from
// reading the next entry in the directory.
auto f = distributed_loader::probe_file(db, sstdir.native(), de.name).then([verifier, sstdir, de] (auto entry) {
auto f = distributed_loader::probe_file(db, sstdir.native(), de.name).then([&verifier, sstdir, de] (auto entry) {
if (entry.component == component_type::TemporaryStatistics) {
return remove_file(sstables::sstable::filename(sstdir.native(), entry.ks, entry.cf, entry.version, entry.generation,
entry.format, component_type::TemporaryStatistics));
}

if (verifier->count(entry.generation)) {
if (verifier->at(entry.generation).status == component_status::has_toc_file) {
if (verifier.count(entry.generation)) {
if (verifier.at(entry.generation).status == component_status::has_toc_file) {
fs::path file_path(sstdir / de.name);
if (entry.component == component_type::TOC) {
throw sstables::malformed_sstable_exception("Invalid State encountered. TOC file already processed", file_path.native());
} else if (entry.component == component_type::TemporaryTOC) {
throw sstables::malformed_sstable_exception("Invalid State encountered. Temporary TOC file found after TOC file was processed", file_path.native());
}
} else if (entry.component == component_type::TOC) {
verifier->at(entry.generation).status = component_status::has_toc_file;
verifier.at(entry.generation).status = component_status::has_toc_file;
} else if (entry.component == component_type::TemporaryTOC) {
verifier->at(entry.generation).status = component_status::has_temporary_toc_file;
verifier.at(entry.generation).status = component_status::has_temporary_toc_file;
}
} else {
if (entry.component == component_type::TOC) {
verifier->emplace(entry.generation, sstable_descriptor{component_status::has_toc_file, entry.version, entry.format});
verifier.emplace(entry.generation, sstable_descriptor{component_status::has_toc_file, entry.version, entry.format});
} else if (entry.component == component_type::TemporaryTOC) {
verifier->emplace(entry.generation, sstable_descriptor{component_status::has_temporary_toc_file, entry.version, entry.format});
verifier.emplace(entry.generation, sstable_descriptor{component_status::has_temporary_toc_file, entry.version, entry.format});
} else {
verifier->emplace(entry.generation, sstable_descriptor{component_status::has_some_file, entry.version, entry.format});
verifier.emplace(entry.generation, sstable_descriptor{component_status::has_some_file, entry.version, entry.format});
}
}
return make_ready_future<>();
Expand All @@ -724,8 +723,8 @@ future<> distributed_loader::do_populate_column_family(distributed<database>& db
return make_ready_future<>();
}, &column_family::manifest_json_filter).then([&futures] {
return execute_futures(futures);
}).then([verifier, sstdir, ks = std::move(ks), cf = std::move(cf)] {
return do_for_each(*verifier, [sstdir = std::move(sstdir), ks = std::move(ks), cf = std::move(cf), verifier] (auto v) {
}).then([&verifier, sstdir, ks = std::move(ks), cf = std::move(cf)] {
return do_for_each(verifier, [sstdir = std::move(sstdir), ks = std::move(ks), cf = std::move(cf)] (auto v) {
if (v.second.status == component_status::has_temporary_toc_file) {
unsigned long gen = v.first;
sstables::sstable::version_types version = v.second.version;
Expand Down

0 comments on commit 2d255d0

Please sign in to comment.