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
  • Loading branch information
@bhalevy @avikivity
bhalevy authored and avikivity committed Aug 20, 2023
1 parent e23d9cd commit 0f54e24
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 @@ -123,19 +123,19 @@ 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<> 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<> update_tablet_metadata();
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<> 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);
future<> drop_function(const db::functions::function_name& fun_name, const std::vector<data_type>& arg_types);
future<> drop_aggregate(const db::functions::function_name& fun_name, const std::vector<data_type>& arg_types);

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 @@ -441,7 +441,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 @@ -450,7 +450,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 @@ -460,7 +460,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 @@ -470,7 +470,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 @@ -494,7 +494,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 @@ -503,7 +503,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 @@ -513,7 +513,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 @@ -523,7 +523,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 Down Expand Up @@ -555,15 +555,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 @@ -573,7 +573,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 @@ -583,7 +583,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 0f54e24

Please sign in to comment.