api: prevent non-owner cpu access to shared_ptr #14532
Conversation
|
@scylladb/scylla-maint please review and trigger CI for this pull request |
@benipeled @avikivity do we have a bug in the list of ScyllaDB developers? |
Yes, I assumed that the |
|
Trigger CI manually https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/2281/ |
|
CI state |
api/column_family.cc
Outdated
| return ctx.db.map_reduce0([key, uuid] (replica::database& db) -> future<std::unordered_set<sstring>> { | ||
| auto sstables = co_await db.find_column_family(uuid).get_sstables_by_partition_key(key); | ||
| auto names = sstables | boost::adaptors::transformed([] (auto s) { return s->get_filename(); }); | ||
| co_return std::unordered_set<sstring>(names.begin(), names.end()); |
There was a problem hiding this comment.
This can be achieved more simply in a single pass with boost::copy_range<std::unordered_set>(sstables | boost::adaptors::transformed ...)
api/column_family.cc
Outdated
| auto names = b | boost::adaptors::transformed([] (auto s) { return s->get_filename(); }); | ||
| a.insert(names.begin(), names.end()); | ||
| [](std::unordered_set<sstring> a, std::unordered_set<sstring>&& b) mutable { | ||
| a.insert(b.begin(), b.end()); |
There was a problem hiding this comment.
a.merge(b) may be slightly more effcient (and shorter)
|
Since when the regression? |
In get_sstables_for_key in api/column_family.cc a set of lw_shared_ptrs to sstables is passes to reducer of map_reduce0. Reducer then accesses these shared pointers. As reducer is invoked on the same shard map_reduce0 is called, we have an illegal access to shared pointer on non-owner cpu. A set of shared pointers to sstables is trasnsformed in map function, which is guaranteed to be invoked on a shard associated with the service. Fixes: scylladb#14515.
|
Regression since 198bca9. |
|
|
CI state |
|
@scylladb/scylla-maint please review/merge. |
|
Absolutely, this bug has a very low probability of triggering a very nasty bug. |
In get_sstables_for_key in api/column_family.cc a set of lw_shared_ptrs to sstables is passes to reducer of map_reduce0. Reducer then accesses these shared pointers. As reducer is invoked on the same shard map_reduce0 is called, we have an illegal access to shared pointer on non-owner cpu.
A set of shared pointers to sstables is trasnsformed in map function, which is guaranteed to be invoked on a shard associated with the service.
Fixes: #14515.