Skip to content

Commit

Permalink
migration_notifier: get schema_ptr by value
Browse files Browse the repository at this point in the history 
To prevent use-after-free as seen in
scylladb#15097
where a temp schema_ptr retrieved from a global_schema_ptr
get destroyed when the notification function yielded.

Capturing the schema_ptr on the coroutine frame
is inexpensive since its a shared ptr and it makes sure
that the schema remains valid throughput the coroutine
life time.

\Fixes scylladb#15097

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>

\Closes scylladb#15098

(cherry picked from commit 0f54e24)
  • Loading branch information
@bhalevy
bhalevy committed Oct 29, 2023
1 parent 8064ffa commit cd7abb3
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 24 deletions.
24 changes: 12 additions & 12 deletions service/migration_listener.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,18 +115,18 @@ public:
/// Unregister a migration listener on current shard.
future<> unregister_listener(migration_listener* listener);

future<> create_keyspace(const lw_shared_ptr<keyspace_metadata>& ksm);
future<> create_column_family(const schema_ptr& cfm);
future<> create_user_type(const user_type& type);
future<> create_view(const view_ptr& view);
future<> update_keyspace(const lw_shared_ptr<keyspace_metadata>& ksm);
future<> update_column_family(const schema_ptr& cfm, bool columns_changed);
future<> update_user_type(const user_type& type);
future<> update_view(const view_ptr& view, bool columns_changed);
future<> drop_keyspace(const sstring& ks_name);
future<> drop_column_family(const schema_ptr& cfm);
future<> drop_user_type(const user_type& type);
future<> drop_view(const view_ptr& view);
future<> create_keyspace(lw_shared_ptr<keyspace_metadata> ksm);
future<> create_column_family(schema_ptr cfm);
future<> create_user_type(user_type type);
future<> create_view(view_ptr view);
future<> update_keyspace(lw_shared_ptr<keyspace_metadata> ksm);
future<> update_column_family(schema_ptr cfm, bool columns_changed);
future<> update_user_type(user_type type);
future<> update_view(view_ptr view, bool columns_changed);
future<> drop_keyspace(sstring ks_name);
future<> drop_column_family(schema_ptr cfm);
future<> drop_user_type(user_type type);
future<> drop_view(view_ptr view);

void before_create_column_family(const schema&, std::vector<mutation>&, api::timestamp_type);
void before_update_column_family(const schema& new_schema, const schema& old_schema, std::vector<mutation>&, api::timestamp_type);
Expand Down
24 changes: 12 additions & 12 deletions service/migration_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ future<> migration_notifier::on_schema_change(std::function<void(migration_liste
});
}

future<> migration_notifier::create_keyspace(const lw_shared_ptr<keyspace_metadata>& ksm) {
future<> migration_notifier::create_keyspace(lw_shared_ptr<keyspace_metadata> ksm) {
const auto& name = ksm->name();
co_await on_schema_change([&] (migration_listener* listener) {
listener->on_create_keyspace(name);
Expand All @@ -400,7 +400,7 @@ future<> migration_notifier::create_keyspace(const lw_shared_ptr<keyspace_metada
});
}

future<> migration_notifier::create_column_family(const schema_ptr& cfm) {
future<> migration_notifier::create_column_family(schema_ptr cfm) {
const auto& ks_name = cfm->ks_name();
const auto& cf_name = cfm->cf_name();
co_await on_schema_change([&] (migration_listener* listener) {
Expand All @@ -410,7 +410,7 @@ future<> migration_notifier::create_column_family(const schema_ptr& cfm) {
});
}

future<> migration_notifier::create_user_type(const user_type& type) {
future<> migration_notifier::create_user_type(user_type type) {
const auto& ks_name = type->_keyspace;
const auto& type_name = type->get_name_as_string();
co_await on_schema_change([&] (migration_listener* listener) {
Expand All @@ -420,7 +420,7 @@ future<> migration_notifier::create_user_type(const user_type& type) {
});
}

future<> migration_notifier::create_view(const view_ptr& view) {
future<> migration_notifier::create_view(view_ptr view) {
const auto& ks_name = view->ks_name();
const auto& view_name = view->cf_name();
co_await on_schema_change([&] (migration_listener* listener) {
Expand All @@ -444,7 +444,7 @@ public void notifyCreateAggregate(UDAggregate udf)
}
#endif

future<> migration_notifier::update_keyspace(const lw_shared_ptr<keyspace_metadata>& ksm) {
future<> migration_notifier::update_keyspace(lw_shared_ptr<keyspace_metadata> ksm) {
const auto& name = ksm->name();
co_await on_schema_change([&] (migration_listener* listener) {
listener->on_update_keyspace(name);
Expand All @@ -453,7 +453,7 @@ future<> migration_notifier::update_keyspace(const lw_shared_ptr<keyspace_metada
});
}

future<> migration_notifier::update_column_family(const schema_ptr& cfm, bool columns_changed) {
future<> migration_notifier::update_column_family(schema_ptr cfm, bool columns_changed) {
const auto& ks_name = cfm->ks_name();
const auto& cf_name = cfm->cf_name();
co_await on_schema_change([&] (migration_listener* listener) {
Expand All @@ -463,7 +463,7 @@ future<> migration_notifier::update_column_family(const schema_ptr& cfm, bool co
});
}

future<> migration_notifier::update_user_type(const user_type& type) {
future<> migration_notifier::update_user_type(user_type type) {
const auto& ks_name = type->_keyspace;
const auto& type_name = type->get_name_as_string();
co_await on_schema_change([&] (migration_listener* listener) {
Expand All @@ -473,7 +473,7 @@ future<> migration_notifier::update_user_type(const user_type& type) {
});
}

future<> migration_notifier::update_view(const view_ptr& view, bool columns_changed) {
future<> migration_notifier::update_view(view_ptr view, bool columns_changed) {
const auto& ks_name = view->ks_name();
const auto& view_name = view->cf_name();
co_await on_schema_change([&] (migration_listener* listener) {
Expand All @@ -497,15 +497,15 @@ public void notifyUpdateAggregate(UDAggregate udf)
}
#endif

future<> migration_notifier::drop_keyspace(const sstring& ks_name) {
future<> migration_notifier::drop_keyspace(sstring ks_name) {
co_await on_schema_change([&] (migration_listener* listener) {
listener->on_drop_keyspace(ks_name);
}, [&] (std::exception_ptr ex) {
return fmt::format("Drop keyspace notification failed {}: {}", ks_name, ex);
});
}

future<> migration_notifier::drop_column_family(const schema_ptr& cfm) {
future<> migration_notifier::drop_column_family(schema_ptr cfm) {
const auto& ks_name = cfm->ks_name();
const auto& cf_name = cfm->cf_name();
co_await on_schema_change([&] (migration_listener* listener) {
Expand All @@ -515,7 +515,7 @@ future<> migration_notifier::drop_column_family(const schema_ptr& cfm) {
});
}

future<> migration_notifier::drop_user_type(const user_type& type) {
future<> migration_notifier::drop_user_type(user_type type) {
const auto& ks_name = type->_keyspace;
const auto& type_name = type->get_name_as_string();
co_await on_schema_change([&] (migration_listener* listener) {
Expand All @@ -525,7 +525,7 @@ future<> migration_notifier::drop_user_type(const user_type& type) {
});
}

future<> migration_notifier::drop_view(const view_ptr& view) {
future<> migration_notifier::drop_view(view_ptr view) {
const auto& ks_name = view->ks_name();
const auto& view_name = view->cf_name();
co_await on_schema_change([&] (migration_listener* listener) {
Expand Down

0 comments on commit cd7abb3

Please sign in to comment.