frontend: fix leaving FE canvases in bad state

The `obs_frontend_remove_canvas` is called, the
destructor of OBS::Canvas calls
obs_canvas_release() on an object which is
both allocated on the heap and used as reference.

This causes the `canvases` collection on the main
window to become corrupt, and crash the next time
you try to add another frontend canvas.

This PR modifies the canvases vector to be
represented as a map of obs_canvas_t* to a
shared pointer to OBS::Canvas.

This will preserve both the OBS::Canvas manager
instance, and pointer to obs_canvas_t* for
reference by obs_frontend_canvas_remove(),
ensuring that the right objects will be used
when called upon.

Author: zavitax

frontend/OBSStudioAPI.cpp

@@ -664,8 +664,8 @@ void OBSStudioAPI::obs_frontend_add_undo_redo_action(const char *name, const und
 
 void OBSStudioAPI::obs_frontend_get_canvases(obs_frontend_canvas_list *canvas_list)
 {
-	for (const auto &canvas : main->canvases) {
-		obs_canvas_t *ref = obs_canvas_get_ref(canvas);
+	for (const auto &kv : main->canvases) {
+		obs_canvas_t *ref = obs_canvas_get_ref(kv.first);
 		if (ref)
 			da_push_back(canvas_list->canvases, &ref);
 	}
@@ -674,7 +674,7 @@ void OBSStudioAPI::obs_frontend_get_canvases(obs_frontend_canvas_list *canvas_li
 obs_canvas_t *OBSStudioAPI::obs_frontend_add_canvas(const char *name, obs_video_info *ovi, int flags)
 {
 	auto &canvas = main->AddCanvas(std::string(name), ovi, flags);
-	return obs_canvas_get_ref(canvas);
+	return obs_canvas_get_ref(canvas->canvas);
 }
 
 bool OBSStudioAPI::obs_frontend_remove_canvas(obs_canvas_t *canvas)

frontend/settings/OBSBasicSettings_Stream.cpp

@@ -173,10 +173,11 @@ void OBSBasicSettings::LoadStream1Settings()
 	ui->multitrackVideoAdditionalCanvas->clear();
 	ui->multitrackVideoAdditionalCanvas->addItem(QTStr("None"));
 	for (const auto &canvas : main->GetCanvases()) {
-		if (obs_canvas_get_flags(canvas) & EPHEMERAL)
+		if (obs_canvas_get_flags(canvas.first) & EPHEMERAL)
 			continue;
 
-		ui->multitrackVideoAdditionalCanvas->addItem(obs_canvas_get_name(canvas), obs_canvas_get_uuid(canvas));
+		ui->multitrackVideoAdditionalCanvas->addItem(obs_canvas_get_name(canvas.first),
+							     obs_canvas_get_uuid(canvas.first));
 	}
 
 	if (config_has_user_value(main->Config(), "Stream1", "MultitrackExtraCanvas")) {

frontend/utility/OBSCanvas.cpp

@@ -38,12 +38,14 @@ Canvas::~Canvas() noexcept
 	canvas = nullptr;
 }
 
+/*
 Canvas &Canvas::operator=(Canvas &&other) noexcept
 {
 	canvas = std::exchange(other.canvas, canvas);
 
 	return *this;
 }
+*/
 
 std::optional<OBSDataAutoRelease> Canvas::Save() const
 {
@@ -55,37 +57,37 @@ std::optional<OBSDataAutoRelease> Canvas::Save() const
 	return std::nullopt;
 }
 
-std::unique_ptr<Canvas> Canvas::Load(obs_data_t *data)
+std::shared_ptr<Canvas> Canvas::Load(obs_data_t *data)
 {
 	if (OBSDataAutoRelease canvas_data = obs_data_get_obj(data, "info")) {
 		if (obs_canvas_t *canvas = obs_load_canvas(canvas_data)) {
-			return std::make_unique<Canvas>(canvas);
+			return std::make_shared<Canvas>(canvas);
 		}
 	}
 
 	return nullptr;
 }
 
-std::vector<Canvas> Canvas::LoadCanvases(obs_data_array_t *canvases)
+std::map<obs_canvas_t *, std::shared_ptr<Canvas>> Canvas::LoadCanvases(obs_data_array_t *canvases)
 {
 	auto cb = [](obs_data_t *data, void *param) -> void {
-		auto vec = static_cast<std::vector<Canvas> *>(param);
+		auto vec = static_cast<std::map<obs_canvas_t *, std::shared_ptr<Canvas>> *>(param);
 		if (auto canvas = Canvas::Load(data))
-			vec->emplace_back(std::move(*canvas));
+			(*vec)[canvas->canvas] = canvas;
 	};
 
-	std::vector<Canvas> ret;
+	std::map<obs_canvas_t *, std::shared_ptr<Canvas>> ret;
 	obs_data_array_enum(canvases, cb, &ret);
 
 	return ret;
 }
 
-OBSDataArrayAutoRelease Canvas::SaveCanvases(const std::vector<Canvas> &canvases)
+OBSDataArrayAutoRelease Canvas::SaveCanvases(const std::map<obs_canvas_t *, std::shared_ptr<Canvas>> &canvases)
 {
 	OBSDataArrayAutoRelease savedCanvases = obs_data_array_create();
 
-	for (auto &canvas : canvases) {
-		auto canvas_data = canvas.Save();
+	for (auto kv : canvases) {
+		auto canvas_data = kv.second->Save();
 		if (!canvas_data)
 			continue;
 

frontend/utility/OBSCanvas.hpp

@@ -20,6 +20,7 @@
 #include <memory>
 #include <optional>
 #include <vector>
+#include <map>
 
 #include "obs.h"
 #include "obs.hpp"
@@ -37,16 +38,16 @@ class Canvas {
 	Canvas() = delete;
 	Canvas(Canvas &other) = delete;
 
-	Canvas &operator=(Canvas &&other) noexcept;
+	Canvas &operator=(Canvas &&other) = delete;
 
 	operator obs_canvas_t *() const { return canvas; }
 
 	[[nodiscard]] std::optional<OBSDataAutoRelease> Save() const;
-	static std::unique_ptr<Canvas> Load(obs_data_t *data);
-	static std::vector<Canvas> LoadCanvases(obs_data_array_t *canvases);
-	static OBSDataArrayAutoRelease SaveCanvases(const std::vector<Canvas> &canvases);
+	static std::shared_ptr<Canvas> Load(obs_data_t *data);
+	static std::map<obs_canvas_t *, std::shared_ptr<Canvas>> LoadCanvases(obs_data_array_t *canvases);
+	static OBSDataArrayAutoRelease SaveCanvases(const std::map<obs_canvas_t *, std::shared_ptr<Canvas>> &canvases);
 
-private:
+public:
 	obs_canvas_t *canvas = nullptr;
 };
 } // namespace OBS

frontend/widgets/OBSBasic.hpp

@@ -1120,14 +1120,15 @@ private slots:
 	 * -------------------------------------
 	 */
 private:
-	std::vector<OBS::Canvas> canvases;
+	std::map<obs_canvas_t *, std::shared_ptr<OBS::Canvas>> canvases;
 
 	static void CanvasRemoved(void *data, calldata_t *params);
 
 public:
-	const std::vector<OBS::Canvas> &GetCanvases() const noexcept { return canvases; }
+	const std::map<obs_canvas_t *, std::shared_ptr<OBS::Canvas>> &GetCanvases() const noexcept { return canvases; }
 
-	const OBS::Canvas &AddCanvas(const std::string &name, obs_video_info *ovi = nullptr, int flags = 0);
+	const std::shared_ptr<OBS::Canvas> AddCanvas(const std::string &name, obs_video_info *ovi = nullptr,
+						     int flags = 0);
 
 public slots:
 	bool RemoveCanvas(OBSCanvas canvas);

frontend/widgets/OBSBasic_Canvases.cpp

@@ -23,22 +23,22 @@ void OBSBasic::CanvasRemoved(void *data, calldata_t *params)
 	QMetaObject::invokeMethod(static_cast<OBSBasic *>(data), "RemoveCanvas", Q_ARG(OBSCanvas, OBSCanvas(canvas)));
 }
 
-const OBS::Canvas &OBSBasic::AddCanvas(const std::string &name, obs_video_info *ovi, int flags)
+const std::shared_ptr<OBS::Canvas> OBSBasic::AddCanvas(const std::string &name, obs_video_info *ovi, int flags)
 {
 	OBSCanvas canvas = obs_canvas_create(name.c_str(), ovi, flags);
-	auto &it = canvases.emplace_back(canvas);
+	canvases[canvas] = std::make_shared<OBS::Canvas>(canvas);
+
 	OnEvent(OBS_FRONTEND_EVENT_CANVAS_ADDED);
-	return it;
+	return canvases[canvas];
 }
 
 bool OBSBasic::RemoveCanvas(OBSCanvas canvas)
 {
 	if (!canvas)
 		return false;
 
-	auto canvas_it = std::find(std::begin(canvases), std::end(canvases), canvas);
-	if (canvas_it != std::end(canvases)) {
-		canvases.erase(canvas_it);
+	if (canvases.count(canvas)) {
+		canvases.erase(canvas);
 		OnEvent(OBS_FRONTEND_EVENT_CANVAS_REMOVED);
 		return true;
 	}

frontend/widgets/OBSBasic_SceneCollections.cpp

@@ -1535,8 +1535,8 @@ void OBSBasic::ClearSceneData()
 	obs_enum_scenes(cb, nullptr);
 	obs_enum_sources(cb, nullptr);
 
-	for (const auto &canvas : canvases) {
-		obs_canvas_enum_scenes(canvas, cb, nullptr);
+	for (const auto &kv : canvases) {
+		obs_canvas_enum_scenes(kv.first, cb, nullptr);
 	}
 
 	canvases.clear();