From d68c357be07356221d81e56662d1df5b488b5ad3 Mon Sep 17 00:00:00 2001
From: Goober5000 <ipw47@mailbox.org>
Date: Fri, 12 Jun 2026 01:12:57 -0400
Subject: [PATCH 1/5] harden move operations in w_bank and LuaThread

w_bank: add a move constructor.  object_copy_including_array_member
returns by value, and without a move constructor the return fell back
to the shallow copy constructor whenever NRVO was not applied (e.g.
MSVC debug builds), after which the local's destructor deleted the
freshly allocated arrays -- leaving the stored bank dangling and
double-deleting at model unload.  Also add noexcept to the move
operations, replace the destructor call in move-assignment with
explicit delete[]s (using an object after its lifetime has ended is
undefined behavior), guard against self-move, and document that the
defaulted copy constructor is intentionally shallow for
object_copy_including_array_member's copy-then-replace pattern.

LuaThread: declare the defaulted move operations noexcept.  This
exposed that LuaTable was not movable at all (its user-declared
destructor suppressed the implicit moves), so LuaThread's implicit
move spec was formally throwing -- a hard error on gcc 9, which
still requires the spec on a defaulted redeclaration to match the
implicit one.  Restore LuaTable's rule of five and declare the
LuaValue and LuaFunction copy operations noexcept (they only copy a
shared_ptr, a raw pointer, and an enum), making the noexcept claims
hold throughout the hierarchy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 code/model/model.h                 | 32 +++++++++++++++++++++---------
 code/scripting/lua/LuaFunction.cpp |  4 ++--
 code/scripting/lua/LuaFunction.h   |  4 ++--
 code/scripting/lua/LuaTable.h      |  6 ++++++
 code/scripting/lua/LuaThread.cpp   |  4 ++--
 code/scripting/lua/LuaThread.h     |  4 ++--
 code/scripting/lua/LuaValue.cpp    |  4 ++--
 code/scripting/lua/LuaValue.h      |  6 ++++--
 8 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/code/model/model.h b/code/model/model.h
index 109db237da6..cb10c67aab3 100644
--- a/code/model/model.h
+++ b/code/model/model.h
@@ -15,6 +15,7 @@
 #include "globalincs/globals.h" // for NAME_LENGTH
 #include "globalincs/pstypes.h"
 #include <array>
+#include <utility>
  
 #include "actions/Program.h"
 #include "gamesnd/gamesnd.h"
@@ -563,17 +564,30 @@ struct w_bank
 		delete[] external_model_angle_offset;
 	}
 
-	w_bank& operator=(w_bank&& other) {
-		this->~w_bank();
-		num_slots = other.num_slots;
-		pnt = other.pnt;
-		norm = other.norm;
-		external_model_angle_offset = other.external_model_angle_offset;
-		other.pnt = nullptr;
-		other.norm = nullptr;
-		other.external_model_angle_offset = nullptr;
+	w_bank(w_bank&& other) noexcept
+		: num_slots(std::exchange(other.num_slots, 0)),
+		  pnt(std::exchange(other.pnt, nullptr)),
+		  norm(std::exchange(other.norm, nullptr)),
+		  external_model_angle_offset(std::exchange(other.external_model_angle_offset, nullptr))
+	{}
+
+	w_bank& operator=(w_bank&& other) noexcept {
+		if (this != &other) {
+			delete[] pnt;
+			delete[] norm;
+			delete[] external_model_angle_offset;
+			num_slots = std::exchange(other.num_slots, 0);
+			pnt = std::exchange(other.pnt, nullptr);
+			norm = std::exchange(other.norm, nullptr);
+			external_model_angle_offset = std::exchange(other.external_model_angle_offset, nullptr);
+		}
 		return *this;
 	}
+
+	// The copy constructor is shallow!  It exists for object_copy_including_array_member,
+	// which copy-constructs and then immediately replaces every owned array with a fresh
+	// allocation.  Do not copy-construct a w_bank in any other context, because the
+	// destructor will delete[] the aliased arrays.
 	w_bank(const w_bank& other) = default;
 	w_bank& operator=(const w_bank& other) = delete;
 };
diff --git a/code/scripting/lua/LuaFunction.cpp b/code/scripting/lua/LuaFunction.cpp
index bec9d05fb4f..54e2ba4b8f5 100644
--- a/code/scripting/lua/LuaFunction.cpp
+++ b/code/scripting/lua/LuaFunction.cpp
@@ -110,8 +110,8 @@ LuaFunction LuaFunction::createFromCode(lua_State* L, std::string const& code, s
 LuaFunction::LuaFunction() : LuaValue(), _errorFunction(nullptr) {
 }
 
-LuaFunction::LuaFunction(const LuaFunction&) = default;
-LuaFunction& LuaFunction::operator=(const LuaFunction&) = default;
+LuaFunction::LuaFunction(const LuaFunction&) noexcept = default;
+LuaFunction& LuaFunction::operator=(const LuaFunction&) noexcept = default;
 
 LuaFunction::LuaFunction(LuaFunction&&) noexcept = default;
 LuaFunction& LuaFunction::operator=(LuaFunction&&) noexcept = default;
diff --git a/code/scripting/lua/LuaFunction.h b/code/scripting/lua/LuaFunction.h
index 71e7d93f2ef..c9587712a03 100644
--- a/code/scripting/lua/LuaFunction.h
+++ b/code/scripting/lua/LuaFunction.h
@@ -69,8 +69,8 @@ class LuaFunction: public LuaValue {
      *
      * @param other The other function.
      */
-	LuaFunction(const LuaFunction& other);
-	LuaFunction& operator=(const LuaFunction& other);
+	LuaFunction(const LuaFunction& other) noexcept;
+	LuaFunction& operator=(const LuaFunction& other) noexcept;
 
 	LuaFunction(LuaFunction&&) noexcept;
 	LuaFunction& operator=(LuaFunction&&) noexcept;
diff --git a/code/scripting/lua/LuaTable.h b/code/scripting/lua/LuaTable.h
index 91508884a83..06d177e091b 100644
--- a/code/scripting/lua/LuaTable.h
+++ b/code/scripting/lua/LuaTable.h
@@ -110,6 +110,12 @@ class LuaTable: public LuaValue {
      */
 	~LuaTable() override;
 
+	// Since we have a user-declared destructor, we need these too.
+	LuaTable(const LuaTable&) noexcept = default;
+	LuaTable& operator=(const LuaTable&) noexcept = default;
+	LuaTable(LuaTable&&) noexcept = default;
+	LuaTable& operator=(LuaTable&&) noexcept = default;
+
 	/**
      * @brief Sets the metatable.
      *
diff --git a/code/scripting/lua/LuaThread.cpp b/code/scripting/lua/LuaThread.cpp
index 48eb14b7979..3e67fde2acc 100644
--- a/code/scripting/lua/LuaThread.cpp
+++ b/code/scripting/lua/LuaThread.cpp
@@ -68,8 +68,8 @@ LuaThread LuaThread::create(lua_State* L, const LuaFunction& func)
 LuaThread::LuaThread() = default;
 LuaThread::LuaThread(lua_State* luaState, lua_State* thread) : LuaValue(luaState), _thread(thread) {}
 
-LuaThread::LuaThread(LuaThread&&) = default;
-LuaThread& LuaThread::operator=(LuaThread&&) = default;
+LuaThread::LuaThread(LuaThread&&) noexcept = default;
+LuaThread& LuaThread::operator=(LuaThread&&) noexcept = default;
 
 LuaThread::~LuaThread() = default;
 
diff --git a/code/scripting/lua/LuaThread.h b/code/scripting/lua/LuaThread.h
index 17eb9f758b5..7f5d36c70e7 100644
--- a/code/scripting/lua/LuaThread.h
+++ b/code/scripting/lua/LuaThread.h
@@ -43,8 +43,8 @@ class LuaThread : public LuaValue {
 	LuaThread(const LuaThread&) = delete;
 	LuaThread& operator=(const LuaThread&) = delete;
 
-	LuaThread(LuaThread&&); 
-	LuaThread& operator=(LuaThread&&);
+	LuaThread(LuaThread&&) noexcept;
+	LuaThread& operator=(LuaThread&&) noexcept;
 
 	~LuaThread() override;
 
diff --git a/code/scripting/lua/LuaValue.cpp b/code/scripting/lua/LuaValue.cpp
index 13508033b7f..8ad046fa01a 100644
--- a/code/scripting/lua/LuaValue.cpp
+++ b/code/scripting/lua/LuaValue.cpp
@@ -52,8 +52,8 @@ LuaValue::LuaValue(lua_State* state) : _luaState(state)
 	Assertion(state != nullptr, "Lua state pointer is not valid!");
 }
 
-LuaValue::LuaValue(const LuaValue&) = default;
-LuaValue& LuaValue::operator=(const LuaValue&) = default;
+LuaValue::LuaValue(const LuaValue&) noexcept = default;
+LuaValue& LuaValue::operator=(const LuaValue&) noexcept = default;
 
 LuaValue::LuaValue(LuaValue&&) noexcept = default;
 LuaValue& LuaValue::operator=(LuaValue&&) noexcept = default;
diff --git a/code/scripting/lua/LuaValue.h b/code/scripting/lua/LuaValue.h
index 24ee3603622..1ab67b038d8 100644
--- a/code/scripting/lua/LuaValue.h
+++ b/code/scripting/lua/LuaValue.h
@@ -80,9 +80,11 @@ class LuaValue {
 	/**
 	 * @brief Copy-constructor
 	 * @param other The other LuaValue.
+	 * @note The noexcept is a promise that all members remain nothrow-copyable
+	 * (currently a shared_ptr, a raw pointer, and an enum).
 	 */
-	LuaValue(const LuaValue& other);
-	LuaValue& operator=(const LuaValue& other);
+	LuaValue(const LuaValue& other) noexcept;
+	LuaValue& operator=(const LuaValue& other) noexcept;
 
 	/**
 	 * @brief Move-constructor

From b14ca134cc2ac258bf8076201fa77a1d0541809a Mon Sep 17 00:00:00 2001
From: Goober5000 <ipw47@mailbox.org>
Date: Fri, 12 Jun 2026 02:59:03 -0400
Subject: [PATCH 2/5] fix four more shallow-ownership hazards in by-value types

A sweep for the w_bank pattern (raw owning handles in types that are
copied, moved, or shifted by value) found four more instances:

p_object: the destructor frees dock_list, which suppresses the implicit
move operations, so Parse_objects growth copied elements -- safe only
because dock lists happen to be built after parsing finishes.  Wrap
dock_list in the new util::reset_on_move<> (a handle whose move resets
the source) and explicitly default the rule of five (with a user-defined
move assignment that frees any existing dock list before the memberwise
transfer), so moves now transfer the dock list.

DecalDefinition: the defaulted moves copied the bitmap handles without
resetting the source, so destroying a moved-from element would
bm_release live handles -- safe only because DecalDefinitions grows
before loadBitmaps runs.  Wrap the handles in reset_on_move<int, -1>.

cmission: FRED's remove_mission overwrote the removed slot's five
vm_strdup'd strings without freeing them (a leak on every removal) and
left the vacated slot aliasing the surviving copy (a double-free if the
slot is later reused without reassigning every string).  Factor the
string cleanup out of mission_campaign_clear into
mission_campaign_free_mission_strings and call it before the overwrite,
nulling the vacated slot afterward.

Undo_stack: the destructor deletes the tracked items, but the implicit
shallow copy was used to transfer stacks into the undo system, kept safe
only by an immediately-following untrack() call.  Delete the copy
operations, default the move constructor and implement move assignment
as clear-then-exchange, so an assigned-over stack deletes its own items
first and the source is guaranteed empty.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 code/controlconfig/controlsconfig.cpp | 12 +++---
 code/decals/decals.h                  | 10 +++--
 code/globalincs/undosys.cpp           | 30 ++++++++------
 code/globalincs/undosys.h             | 30 +++++++-------
 code/mission/missioncampaign.cpp      | 55 ++++++++++++++------------
 code/mission/missioncampaign.h        |  4 ++
 code/mission/missionparse.cpp         | 13 ++++++
 code/mission/missionparse.h           | 15 ++++++-
 code/source_groups.cmake              |  1 +
 code/utils/reset_on_move.h            | 57 +++++++++++++++++++++++++++
 fred2/campaigntreeview.cpp            | 13 ++++++
 11 files changed, 179 insertions(+), 61 deletions(-)
 create mode 100644 code/utils/reset_on_move.h

diff --git a/code/controlconfig/controlsconfig.cpp b/code/controlconfig/controlsconfig.cpp
index 1dbdc7b3efb..abca44170a2 100644
--- a/code/controlconfig/controlsconfig.cpp
+++ b/code/controlconfig/controlsconfig.cpp
@@ -739,7 +739,7 @@ void control_config_bind(int i, const CC_bind &new_bind, selItem order, bool API
 	Undo_stack stack;
 	stack.save(Control_config[i].first);
 	stack.save(Control_config[i].second);
-	Undo_controls.save_stack(stack);
+	Undo_controls.save_stack(std::move(stack));
 
 	CCB old(Control_config[i]);
 
@@ -770,7 +770,7 @@ bool control_config_remove_binding(int ctrl, selItem item, bool API_Access)
 
 			stack.save(Control_config[ctrl].first);
 			stack.save(Control_config[ctrl].second);
-			Undo_controls.save_stack(stack);
+			Undo_controls.save_stack(std::move(stack));
 
 			Control_config[ctrl].first.clear();
 			Control_config[ctrl].second.clear();
@@ -873,7 +873,7 @@ bool control_config_clear_other(int ctrl, bool API_Access)
 		return false;
 	}
 
-	Undo_controls.save_stack(stack);
+	Undo_controls.save_stack(std::move(stack));
 	control_config_conflict_check();
 
 	if (!API_Access) {
@@ -911,7 +911,7 @@ bool control_config_clear_all(bool API_Access)
 		return false;
 	}
 
-	Undo_controls.save_stack(stack);
+	Undo_controls.save_stack(std::move(stack));
 	control_config_conflict_check();
 
 	if (!API_Access) {
@@ -975,7 +975,7 @@ bool control_config_do_reset(bool cycle, bool API_Access)
 		stack.save(item.first);
 		stack.save(item.second);
 	}
-	Undo_controls.save_stack(stack);
+	Undo_controls.save_stack(std::move(stack));
 
 	control_config_use_preset(Control_config_presets[Defaults_cycle_pos]);
 
@@ -1097,7 +1097,7 @@ bool control_config_toggle_modifier(int bit, int ctrl, bool API_Access)
 	Undo_stack stack;
 	stack.save(Control_config[ctrl].first);
 	stack.save(Control_config[ctrl].second);
-	Undo_controls.save_stack(stack);
+	Undo_controls.save_stack(std::move(stack));
 
 	Control_config[ctrl].take(CC_bind(CID_KEYBOARD, static_cast<short>(k ^ bit)), -1);
 	control_config_conflict_check();
diff --git a/code/decals/decals.h b/code/decals/decals.h
index f7e08138a95..bc0295521db 100644
--- a/code/decals/decals.h
+++ b/code/decals/decals.h
@@ -5,6 +5,7 @@
 #include "object/object.h"
 
 #include "utils/RandomRange.h"
+#include "utils/reset_on_move.h"
 
 namespace decals {
 
@@ -20,9 +21,12 @@ class DecalDefinition {
 	SCP_string _normalMapFilename;
 	bool _loopNormal = true;
 
-	int _diffuseBitmap = -1;
-	int _glowBitmap = -1;
-	int _normalBitmap = -1;
+	// reset_on_move so that the defaulted move operations below transfer the
+	// handles; the destructor bm_release()s them, so a moved-from element must
+	// not keep aliasing live handles
+	util::reset_on_move<int, -1> _diffuseBitmap;
+	util::reset_on_move<int, -1> _glowBitmap;
+	util::reset_on_move<int, -1> _normalBitmap;
 
  public:
 	explicit DecalDefinition(SCP_string name);
diff --git a/code/globalincs/undosys.cpp b/code/globalincs/undosys.cpp
index 9c66626deef..3f76be14bd4 100644
--- a/code/globalincs/undosys.cpp
+++ b/code/globalincs/undosys.cpp
@@ -6,6 +6,8 @@
 #include "globalincs/undosys.h"
 #include "globalincs/vmallocator.h"
 
+#include <utility>
+
 // Pure virtual deconstructors must be defined.
 Undo_item_base::~Undo_item_base() = default;
 
@@ -47,22 +49,20 @@ void Undo_system::clear_redo() {
 	redo_stack.clear();
 }
 
-size_t Undo_system::save_stack(Undo_stack& stack) {
+size_t Undo_system::save_stack(Undo_stack&& stack) {
 	if (stack.size() == 0) {
 		return 0;
 	}
-	
+
 	clear_redo();
 
-	// Copy the stack onto the heap, and push it into our undo_stack as a single item
-	Undo_item_base *new_item = new Undo_stack(stack);
+	// Move the stack onto the heap, and push it into our undo_stack as a single item.
+	// The move empties the input stack, so there's only one deletion handler for the items.
+	Undo_item_base *new_item = new Undo_stack(std::move(stack));
 
 	Assert(new_item != nullptr);
 	undo_stack.push_back(new_item);
 
-	// Input stack is told to untrack the items, so that there's only one deletion handler for them
-	stack.untrack();
-
 	clamp_stacks();
 
 	return undo_stack.size();
@@ -129,6 +129,18 @@ Undo_stack::~Undo_stack() {
 	clear();
 }
 
+Undo_stack& Undo_stack::operator=(Undo_stack&& other) noexcept {
+	if (this != &other) {
+		// delete any items we currently track, since the vector assignment below would otherwise leak them
+		clear();
+
+		reverse = other.reverse;
+		// exchange rather than move, to guarantee the source is left empty and never deletes the items
+		stack = std::exchange(other.stack, {});
+	}
+	return *this;
+}
+
 void Undo_stack::reserve(size_t size) {
 	stack.reserve(size);
 }
@@ -154,10 +166,6 @@ size_t Undo_stack::size() {
 	return stack.size();
 }
 
-void Undo_stack::untrack() {
-	stack.clear();
-}
-
 void Undo_stack::clear() {
 	for (auto &element : stack) {
 		delete element;
diff --git a/code/globalincs/undosys.h b/code/globalincs/undosys.h
index 0d9d4494813..5b66a834cfe 100644
--- a/code/globalincs/undosys.h
+++ b/code/globalincs/undosys.h
@@ -53,7 +53,7 @@ Lastly, you can make stacks of undo operations, so that a single op in the syste
 		for (int i = 0; i < JOY_AXIS_ACTIONS; ++i) {
 			stack.save(Axis_map_to[i], Axis_map_to);		// This example saves a C style array. Does the same thing without a wrapper, but wasteful
 		
-		Undo_controls.save_stack(stack) // Save the stack as a single item!
+		Undo_controls.save_stack(std::move(stack)) // Save the stack as a single item!  (the stack is empty afterwards)
 	}
 
 	std::pair<const void*, const void*> ref = Undo_controls.undo();
@@ -161,6 +161,14 @@ class Undo_stack : public Undo_item_base
 
 	~Undo_stack() override;
 
+	// The destructor deletes the tracked items, so a copy would double-delete;
+	// a stack is transferred into the undo system by move, which empties the
+	// source so that only one owner ever deletes the items.
+	Undo_stack(const Undo_stack&) = delete;
+	Undo_stack& operator=(const Undo_stack&) = delete;
+	Undo_stack(Undo_stack&&) noexcept = default;
+	Undo_stack& operator=(Undo_stack&&) noexcept;
+
 	/*!
 	 * @brief Restores all items within the undo stack
 	 *
@@ -202,15 +210,6 @@ class Undo_stack : public Undo_item_base
 	  */
 	 void clear();
 
-protected:
-	friend class Undo_system;
-
-	/*!
-	 * @brief Calls ::clear() on the internal vector
-	 * @note Caution: This does not delete the tracked Undo_items
-	 */
-	void untrack();
-
 private:
 	bool reverse;   // Direction to walk the stack. forward = false, reverse = True
 
@@ -259,13 +258,14 @@ class Undo_system
 	/*!
 	 * @brief Saves a stack of undo-items as a single undo-item within the system
 	 *
-	 * @param[in,out] stack  The undo stack to save to the undo system.  Data in the stack is "unspecified" after this operation
+	 * @param[in,out] stack  The undo stack to save to the undo system.  Must be passed with std::move(); the
+	 *  stack is empty after this operation
 	 *
-	 * @details  The undo system effectively moves the stack into its internal containers, claiming ownership of the
-	 *  Undo_items and deleting them upon going out of scope.  The input stack is told to untrack the Undo_items in
-	 *  the process, so that there is only ever one reference to the Undo_item like a std::unique_ptr
+	 * @details  The undo system moves the stack into its internal containers, claiming ownership of the
+	 *  Undo_items and deleting them upon going out of scope.  The move empties the input stack, so that there
+	 *  is only ever one owner of each Undo_item, like a std::unique_ptr
 	 */
-	size_t save_stack(Undo_stack& stack);
+	size_t save_stack(Undo_stack&& stack);
 
 	/*!
 	 * @brief Undo's the last changed item and save the changes into the Redo stack
diff --git a/code/mission/missioncampaign.cpp b/code/mission/missioncampaign.cpp
index 4c60527038f..32a1752495e 100644
--- a/code/mission/missioncampaign.cpp
+++ b/code/mission/missioncampaign.cpp
@@ -1194,6 +1194,35 @@ void mission_campaign_mission_over(bool do_next_mission)
 		mission_campaign_next_mission();			// sets up whatever needs to be set to actually play next mission
 }
 
+void mission_campaign_free_mission_strings(cmission &cm)
+{
+	if (cm.name != nullptr) {
+		vm_free(cm.name);
+		cm.name = nullptr;
+	}
+
+	if (cm.notes != nullptr) {
+		vm_free(cm.notes);
+		cm.notes = nullptr;
+	}
+
+	// the next three are strdup'd return values from parselo.cpp - taylor
+	if (cm.mission_branch_desc != nullptr) {
+		vm_free(cm.mission_branch_desc);
+		cm.mission_branch_desc = nullptr;
+	}
+
+	if (cm.mission_branch_brief_anim != nullptr) {
+		vm_free(cm.mission_branch_brief_anim);
+		cm.mission_branch_brief_anim = nullptr;
+	}
+
+	if (cm.mission_branch_brief_sound != nullptr) {
+		vm_free(cm.mission_branch_brief_sound);
+		cm.mission_branch_brief_sound = nullptr;
+	}
+}
+
 /**
  * Called when the game closes -- to get rid of memory errors for Bounds checker
  * also called at campaign init and campaign load
@@ -1207,36 +1236,12 @@ void mission_campaign_clear()
 	// be sure to remove all old malloced strings of Mission_names
 	// we must also free any goal stuff that was from a previous campaign
 	for ( i=0; i<Campaign.num_missions; i++ ) {
-		if ( Campaign.missions[i].name != NULL ) {
-			vm_free(Campaign.missions[i].name);
-			Campaign.missions[i].name = NULL;
-		}
-
-		if (Campaign.missions[i].notes != NULL) {
-			vm_free(Campaign.missions[i].notes);
-			Campaign.missions[i].notes = NULL;
-		}
+		mission_campaign_free_mission_strings(Campaign.missions[i]);
 
 		Campaign.missions[i].goals.clear();
 		Campaign.missions[i].events.clear();
 		Campaign.missions[i].variables.clear();
 
-		// the next three are strdup'd return values from parselo.cpp - taylor
-		if (Campaign.missions[i].mission_branch_desc != NULL) {
-			vm_free(Campaign.missions[i].mission_branch_desc);
-			Campaign.missions[i].mission_branch_desc = NULL;
-		}
-
-		if (Campaign.missions[i].mission_branch_brief_anim != NULL) {
-			vm_free(Campaign.missions[i].mission_branch_brief_anim);
-			Campaign.missions[i].mission_branch_brief_anim = NULL;
-		}
-
-		if (Campaign.missions[i].mission_branch_brief_sound != NULL) {
-			vm_free(Campaign.missions[i].mission_branch_brief_sound);
-			Campaign.missions[i].mission_branch_brief_sound = NULL;
-		}
-
 		if ( !Fred_running ){
 			sexp_unmark_persistent(Campaign.missions[i].formula);		// free any sexpression nodes used by campaign.
 		}
diff --git a/code/mission/missioncampaign.h b/code/mission/missioncampaign.h
index 6e9e6658e48..ba695af8dec 100644
--- a/code/mission/missioncampaign.h
+++ b/code/mission/missioncampaign.h
@@ -195,6 +195,10 @@ extern void mission_campaign_mission_over( bool do_next_mission = true );
 // frees all memory at game close time
 extern void mission_campaign_clear( void );
 
+// frees and nulls a mission's five vm_strdup'd strings (name, notes, and the
+// three mission-branch strings)
+extern void mission_campaign_free_mission_strings(cmission &cm);
+
 // used by Fred to get a mission's list of goals.
 void read_mission_goal_list(int num);
 
diff --git a/code/mission/missionparse.cpp b/code/mission/missionparse.cpp
index bb5185f8b3c..4e7f97b2271 100644
--- a/code/mission/missionparse.cpp
+++ b/code/mission/missionparse.cpp
@@ -3366,6 +3366,19 @@ p_object::~p_object()
 	dock_free_dock_list(this);
 }
 
+p_object &p_object::operator=(p_object &&other) noexcept
+{
+	if (this != &other) {
+		// free our dock list, which the memberwise assignment below would otherwise overwrite (and leak)
+		dock_free_dock_list(this);
+
+		// shallow memberwise copy, then transfer ownership of the dock list by nulling the source's handle
+		*this = other;
+		other.dock_list = nullptr;
+	}
+	return *this;
+}
+
 const char* p_object::get_display_name() {
 	if (has_display_name()) {
 		return display_name.c_str();
diff --git a/code/mission/missionparse.h b/code/mission/missionparse.h
index 50f70f9820f..8e917787108 100644
--- a/code/mission/missionparse.h
+++ b/code/mission/missionparse.h
@@ -23,6 +23,7 @@
 #include "object/object.h"
 #include "parse/sexp.h"
 #include "sound/sound.h"
+#include "utils/reset_on_move.h"
 #include "mission/mission_flags.h"
 #include "nebula/volumetrics.h"
 #include "ship/anchor_t.h"
@@ -480,7 +481,7 @@ class p_object
 	int	score = 0;
 	float assist_score_pct = 0.0f;					// percentage of the score which players who gain an assist will get when this ship is killed
 	SCP_set<size_t> orders_accepted;		// which orders this ship will accept from the player
-	p_dock_instance	*dock_list = nullptr;				// Goober5000 - parse objects this parse object is docked to
+	util::reset_on_move<p_dock_instance *> dock_list;	// Goober5000 - parse objects this parse object is docked to
 	object *created_object = nullptr;					// Goober5000
 	int collision_group_id = 0;							// Goober5000
 	int	group = -1;								// group object is within or -1 if none.
@@ -524,6 +525,18 @@ class p_object
 
 	~p_object();
 
+	// The destructor frees dock_list, and a user-declared destructor suppresses
+	// the implicit move operations, so define them here.  Shallow copies are safe
+	// because parse code only copies p_objects before dock lists are built.
+	p_object() = default;
+	p_object(const p_object &) = default;
+	p_object &operator=(const p_object &) = default;
+	p_object(p_object &&) = default;
+
+	// not defaulted, because a memberwise move would overwrite (and leak) any
+	// dock list the assigned-to object owns; defined in missionparse.cpp
+	p_object &operator=(p_object &&other) noexcept;
+
 	const char* get_display_name();
 	bool has_display_name();
 };
diff --git a/code/source_groups.cmake b/code/source_groups.cmake
index d1483a75dbc..10f08a2a07b 100644
--- a/code/source_groups.cmake
+++ b/code/source_groups.cmake
@@ -1747,6 +1747,7 @@ add_file_folder("Utils"
 	utils/Random.cpp
 	utils/Random.h
 	utils/RandomRange.h
+	utils/reset_on_move.h
 	utils/string_utils.cpp
 	utils/string_utils.h
 	utils/table_viewer.cpp
diff --git a/code/utils/reset_on_move.h b/code/utils/reset_on_move.h
new file mode 100644
index 00000000000..dd5a1a1b319
--- /dev/null
+++ b/code/utils/reset_on_move.h
@@ -0,0 +1,57 @@
+#pragma once
+
+#include <utility>
+
+namespace util
+{
+
+/**
+ * @brief A trivially-copyable value whose MOVE resets the source to a sentinel.
+ *
+ * Intended for raw owning handles (heap pointers, bitmap/sound handles) inside
+ * structs that are stored by value in containers.  An implicitly-generated move
+ * of such a struct copies raw handles without resetting the source, so a
+ * moved-from element still aliases the live resource and a later destructor
+ * releases it twice.  Wrapping the handle in reset_on_move<> makes the
+ * containing struct's implicit (or defaulted) move operations transfer
+ * ownership correctly, without hand-writing memberwise moves.
+ *
+ * Copies remain shallow by design: the wrapper does not know how to duplicate
+ * the underlying resource.  Types whose copies must deep-copy (or never copy)
+ * should delete or implement their own copy operations.
+ *
+ * @tparam T        the handle type (e.g. some_type*, int)
+ * @tparam NullVal  the sentinel a moved-from handle is reset to
+ */
+template <typename T, T NullVal = T{}>
+struct reset_on_move
+{
+	T value = NullVal;
+
+	reset_on_move() = default;
+	reset_on_move(T v) : value(v) {}
+
+	reset_on_move(const reset_on_move &) = default;
+	reset_on_move &operator=(const reset_on_move &) = default;
+
+	reset_on_move(reset_on_move &&other) noexcept : value(std::exchange(other.value, NullVal)) {}
+
+	reset_on_move &operator=(reset_on_move &&other) noexcept
+	{
+		value = std::exchange(other.value, NullVal);
+		return *this;
+	}
+
+	reset_on_move &operator=(T v)
+	{
+		value = v;
+		return *this;
+	}
+
+	operator T() const { return value; }
+
+	// only valid (and only instantiated) when T is a pointer type
+	T operator->() const { return value; }
+};
+
+} // namespace util
diff --git a/fred2/campaigntreeview.cpp b/fred2/campaigntreeview.cpp
index 876dde1e4bc..7932d59692e 100644
--- a/fred2/campaigntreeview.cpp
+++ b/fred2/campaigntreeview.cpp
@@ -15,6 +15,7 @@
 #include "CampaignTreeView.h"
 #include "CampaignEditorDlg.h"
 #include "CampaignTreeWnd.h"
+#include "mission/missioncampaign.h"
 #include "mission/missionparse.h"
 
 #ifdef _DEBUG
@@ -1292,7 +1293,19 @@ void campaign_tree_view::remove_mission(int m)
 	}
 
 	Elements[m] = Elements[z];
+
+	// free the removed mission's strings before its slot is overwritten, then
+	// null the vacated slot's pointers, which now alias slot m's strings.  (If
+	// m == z, the assignment is a self-assign of freed pointers, and the
+	// nulling below leaves the slot clean.)
+	mission_campaign_free_mission_strings(Campaign.missions[m]);
 	Campaign.missions[m] = Campaign.missions[z];
+	Campaign.missions[z].name = nullptr;
+	Campaign.missions[z].notes = nullptr;
+	Campaign.missions[z].mission_branch_desc = nullptr;
+	Campaign.missions[z].mission_branch_brief_anim = nullptr;
+	Campaign.missions[z].mission_branch_brief_sound = nullptr;
+
 	if (m == Cur_campaign_mission) {
 		Cur_campaign_mission = -1;
 		box = (CEdit *) Campaign_tree_formp->GetDlgItem(IDC_HELP_BOX);

From 0ba4a719f16c4bf7e2977cb6a644aef2f97d1d4d Mon Sep 17 00:00:00 2001
From: Goober5000 <ipw47@mailbox.org>
Date: Fri, 12 Jun 2026 03:06:04 -0400
Subject: [PATCH 3/5] remove CombinedVariable code, which was never finished
 and is not called anywhere

---
 code/ship/shipfx.cpp | 296 -------------------------------------------
 1 file changed, 296 deletions(-)

diff --git a/code/ship/shipfx.cpp b/code/ship/shipfx.cpp
index ef5b30531a0..845ff3c1f0b 100644
--- a/code/ship/shipfx.cpp
+++ b/code/ship/shipfx.cpp
@@ -2843,302 +2843,6 @@ void shipfx_stop_engine_wash_sound()
 	}
 }
 
-class CombinedVariable
-{
-public:
-	static const int TYPE_NONE;
-	static const int TYPE_FLOAT;
-	static const int TYPE_IMAGE;
-	static const int TYPE_INT;
-	static const int TYPE_SOUND;
-	static const int TYPE_STRING;
-private:
-	int Type;
-	float	su_Float;
-	int		su_Image;
-	int		su_Int;
-	gamesnd_id su_Sound;
-	char	*su_String;
-public:
-	//TYPE_NONE
-	CombinedVariable();
-	//TYPE_FLOAT
-	CombinedVariable(float n_Float);
-	//TYPE_INT
-	CombinedVariable(int n_Int);
-	//TYPE_IMAGE
-	CombinedVariable(int n_Int, ubyte type_override);
-	//TYPE_SOUND
-	CombinedVariable(gamesnd_id n_snd);
-	//TYPE_STRING
-	CombinedVariable(char *n_String);
-	//All types
-	~CombinedVariable();
-
-	//Returns 1 if buffer was successfully written to
-	int getFloat(float *output);
-	//Returns handle or < 0 on failure/wrong type
-	int getHandle();
-	//Returns handle, or < 0 on failure/wrong type
-	int getImage();
-	//Returns 1 if buffer was successfully written to
-	int getInt(int *output);
-	//Returns handle, or < 0 on failure/wrong type
-	gamesnd_id getSound();
-	//Returns 1 if buffer was successfully written to (output_max includes the null terminator)
-	int getString(char *output, size_t output_max);
-
-	//Returns true if TYPE_NONE
-	bool isEmpty();
-};
-
-//Workaround for MSVC6
-const int CombinedVariable::TYPE_NONE=0;
-const int CombinedVariable::TYPE_FLOAT = 1;
-const int CombinedVariable::TYPE_IMAGE = 2;
-const int CombinedVariable::TYPE_INT = 3;
-const int CombinedVariable::TYPE_SOUND = 4;
-const int CombinedVariable::TYPE_STRING  = 5;
-
-//Member functions
-CombinedVariable::CombinedVariable()
-{
-	Type = TYPE_NONE;
-}
-
-CombinedVariable::CombinedVariable(float n_Float)
-{
-	Type = TYPE_FLOAT;
-	su_Float = n_Float;
-}
-
-CombinedVariable::CombinedVariable(int n_Int)
-{
-	Type = TYPE_INT;
-	su_Int = n_Int;
-}
-
-CombinedVariable::CombinedVariable(int n_Int, ubyte type_override)
-{
-	if(type_override == TYPE_IMAGE)
-	{
-		Type = TYPE_IMAGE;
-		su_Image = n_Int;
-	}
-	else
-	{
-		Type = TYPE_INT;
-		su_Int = n_Int;
-	}
-}
-CombinedVariable::CombinedVariable(gamesnd_id n_snd) {
-	Type = TYPE_SOUND;
-	su_Sound = n_snd;
-}
-
-CombinedVariable::CombinedVariable(char *n_String)
-{
-	Type = TYPE_STRING;
-	su_String = (char *)vm_malloc(strlen(n_String)+1);
-	strcpy(su_String, n_String);
-}
-
-CombinedVariable::~CombinedVariable()
-{
-	if(Type == TYPE_STRING)
-	{
-		vm_free(su_String);
-	}
-}
-
-int CombinedVariable::getFloat(float *output)
-{
-	if(Type == TYPE_FLOAT)
-	{
-		*output  = su_Float;
-		return 1;
-	}
-	if(Type == TYPE_IMAGE)
-	{
-		*output = i2fl(su_Image);
-		return 1;
-	}
-	if(Type == TYPE_INT)
-	{
-		*output = i2fl(su_Int);
-		return 1;
-	}
-	if(Type == TYPE_STRING)
-	{
-		*output = (float)atof(su_String);
-		return 1;
-	}
-	return 0;
-}
-int CombinedVariable::getHandle()
-{
-	int i = 0;
-	if(this->getInt(&i))
-		return i;
-	else
-		return -1;
-}
-int CombinedVariable::getImage()
-{
-	if(Type == TYPE_IMAGE)
-		return this->getHandle();
-	else
-		return -1;
-}
-int CombinedVariable::getInt(int *output)
-{
-	if(output == NULL)
-		return 0;
-
-	if(Type == TYPE_FLOAT)
-	{
-		*output  = fl2i(su_Float);
-		return 1;
-	}
-	if(Type == TYPE_IMAGE)
-	{
-		*output = su_Image;
-		return 1;
-	}
-	if(Type == TYPE_INT)
-	{
-		*output = su_Int;
-		return 1;
-	}
-	if(Type == TYPE_STRING)
-	{
-		*output = atoi(su_String);
-		return 1;
-	}
-
-	return 0;
-}
-gamesnd_id CombinedVariable::getSound()
-{
-	if(Type == TYPE_SOUND)
-		return su_Sound;
-	else
-		return {};
-}
-int CombinedVariable::getString(char *output, size_t output_max)
-{
-	if(output == NULL || output_max == 0)
-		return 0;
-
-	if(Type == TYPE_FLOAT)
-	{
-		snprintf(output, output_max, "%f", su_Float);
-		return 1;
-	}
-	if(Type == TYPE_IMAGE)
-	{
-		if(bm_is_valid(su_Image))
-			snprintf(output, output_max, "%s", bm_get_filename(su_Image));
-		return 1;
-	}
-	if(Type == TYPE_INT)
-	{
-		snprintf(output, output_max, "%i", su_Int);
-		return 1;
-	}
-	if(Type == TYPE_SOUND)
-	{
-		Error(LOCATION, "Sound CombinedVariables are not supported yet.");
-		/*if(snd_is_valid(su_Sound))
-			snprintf(output, output_max, "%s", snd_get_filename(su_Sound));*/
-		return 1;
-	}
-	if(Type == TYPE_STRING)
-	{
-		strncpy(output, su_String, output_max);
-		return 1;
-	}
-	return 0;
-}
-bool CombinedVariable::isEmpty()
-{
-	return (Type != TYPE_NONE);
-}
-
-void parse_combined_variable_list(CombinedVariable *dest, flag_def_list *src, size_t num)
-{
-	if(dest == NULL || src == NULL || num == 0)
-		return;
-
-	char buf[NAME_LENGTH*2];
-	buf[sizeof(buf)-1] = '\0';
-
-	flag_def_list *sp = NULL;
-	CombinedVariable *dp = NULL;
-	for(size_t i = 0; i < num; i++)
-	{
-		sp = &src[i];
-		dp = &dest[i];
-
-		snprintf(buf, sizeof(buf), "+%s:", sp->name);
-		if(optional_string(buf))
-		{
-			switch(sp->var)
-			{
-				case CombinedVariable::TYPE_FLOAT:
-				{
-					float f = 0.0f;
-					stuff_float(&f);
-					*dp = CombinedVariable(f);
-					break;
-				}
-				case CombinedVariable::TYPE_INT:
-				{
-					int myInt = 0;
-					stuff_int(&myInt);
-					*dp = CombinedVariable(myInt);
-					break;
-				}
-				case CombinedVariable::TYPE_IMAGE:
-				{
-					char buf2[MAX_FILENAME_LEN];
-					stuff_string(buf2, F_NAME, MAX_FILENAME_LEN);
-					int idx = bm_load(buf2);
-					*dp = CombinedVariable(idx, CombinedVariable::TYPE_IMAGE);
-					break;
-				}
-				case CombinedVariable::TYPE_SOUND:
-				{
-					char buf2[MAX_FILENAME_LEN];
-					stuff_string(buf2, F_NAME, MAX_FILENAME_LEN);
-					auto idx = gamesnd_get_by_name(buf);
-					*dp = CombinedVariable(idx);
-					break;
-				}
-				case CombinedVariable::TYPE_STRING:
-				{
-					char buf2[MAX_NAME_LEN + MAX_FILENAME_LEN];
-					stuff_string(buf2, F_NAME, MAX_FILENAME_LEN+MAX_NAME_LEN);
-					*dp = CombinedVariable(buf2);
-					break;
-				}
-			}
-		}
-	}
-}
-
-#define WV_ANIMATION		0
-#define WV_RADIUS			1
-#define WV_SPEED			2
-#define WV_TIME				3
-
-flag_def_list Warp_variables[] = {
-	{"Animation",		WV_ANIMATION,		CombinedVariable::TYPE_STRING},
-	{"Radius",			WV_RADIUS,			CombinedVariable::TYPE_FLOAT},
-	{"Speed",			WV_SPEED,			CombinedVariable::TYPE_FLOAT},
-	{"Time",			WV_TIME,			CombinedVariable::TYPE_FLOAT},
-};
-
 
 WarpParams::WarpParams()
 {

From f2d643966144da74f520f5da7af0669bc3019d7b Mon Sep 17 00:00:00 2001
From: Goober5000 <ipw47@mailbox.org>
Date: Fri, 12 Jun 2026 13:09:34 -0400
Subject: [PATCH 4/5] store special_message in a unique_ptr, which also makes
 the training_message_queue struct safe for moves

---
 code/mission/missiontraining.cpp | 36 ++++++++++----------------------
 1 file changed, 11 insertions(+), 25 deletions(-)

diff --git a/code/mission/missiontraining.cpp b/code/mission/missiontraining.cpp
index 169dfc14f85..b06aff74ea3 100644
--- a/code/mission/missiontraining.cpp
+++ b/code/mission/missiontraining.cpp
@@ -32,6 +32,7 @@
 #include "sound/fsspeech.h"
 #include "sound/sound.h"
 #include "weapon/emp.h"
+#include "utils/string_utils.h"
 
 
 
@@ -64,7 +65,7 @@ typedef struct {
 	int num;
 	TIMESTAMP timestamp;
 	int length;
-	char *special_message;
+	std::unique_ptr<char[]> special_message;
 } training_message_queue;
 
 SCP_string Training_buf;
@@ -432,7 +433,7 @@ void training_mission_init()
 
 	// Goober5000
 	for (i = 0; i < TRAINING_MESSAGE_QUEUE_MAX; i++)
-		Training_message_queue[i].special_message = NULL;
+		Training_message_queue[i].special_message.reset();
 
 	// only clear player flags if this is actually a training mission
 	if ( The_mission.game_type & MISSION_TYPE_TRAINING ) {
@@ -705,13 +706,7 @@ void training_mission_shutdown()
 
 	// Goober5000
 	for (i = 0; i < TRAINING_MESSAGE_QUEUE_MAX; i++)
-	{
-		if (Training_message_queue[i].special_message != NULL)
-		{
-			vm_free(Training_message_queue[i].special_message);
-			Training_message_queue[i].special_message = NULL;
-		}
-	}
+		Training_message_queue[i].special_message.reset();
 
 	Training_voice = -1;
 	Training_num_lines = Training_obj_num_lines = 0;
@@ -973,13 +968,8 @@ void message_training_queue(const char *text, TIMESTAMP timestamp, int length)
 		Training_message_queue[Training_message_queue_count].timestamp = timestamp;
 		Training_message_queue[Training_message_queue_count].length = length;
 
-		// Goober5000 - this shouldn't happen, but let's be safe
-		if (Training_message_queue[Training_message_queue_count].special_message != NULL)
-		{
-			Int3();
-			vm_free(Training_message_queue[Training_message_queue_count].special_message);
-			Training_message_queue[Training_message_queue_count].special_message = NULL;
-		}
+		// Goober5000 - this should already be freed here, but let's be safe
+		Training_message_queue[Training_message_queue_count].special_message.reset();
 
 		// Goober5000 - replace variables if necessary
 		// karajorma/jg18 - replace container references if necessary
@@ -987,7 +977,7 @@ void message_training_queue(const char *text, TIMESTAMP timestamp, int length)
 		const bool replace_var = sexp_replace_variable_names_with_values(temp_buf);
 		const bool replace_con = sexp_container_replace_refs_with_values(temp_buf);
 		if (replace_var || replace_con)
-			Training_message_queue[Training_message_queue_count].special_message = vm_strdup(temp_buf.c_str());
+			Training_message_queue[Training_message_queue_count].special_message = util::unique_copy(temp_buf.c_str(), false);
 
 		Training_message_queue_count++;
 	}
@@ -1000,22 +990,18 @@ void message_training_remove_from_queue(int idx)
 {
 	// we're overwriting all messages with the next message, but to
 	// avoid memory leaks, we should free the special message entry
-	if (Training_message_queue[idx].special_message != NULL)
-	{
-		vm_free(Training_message_queue[idx].special_message);
-		Training_message_queue[idx].special_message = NULL;
-	}
+	Training_message_queue[idx].special_message.reset();
 
 	// replace current message with the one above it, etc.
 	for (int j=idx+1; j<Training_message_queue_count; j++)
-		Training_message_queue[j - 1] = Training_message_queue[j];
+		Training_message_queue[j - 1] = std::move(Training_message_queue[j]);
 
 	// delete the topmost message
 	Training_message_queue_count--;
 	Training_message_queue[Training_message_queue_count].length = -1;
 	Training_message_queue[Training_message_queue_count].num = -1;
 	Training_message_queue[Training_message_queue_count].timestamp = TIMESTAMP::invalid();
-	Training_message_queue[Training_message_queue_count].special_message = NULL;	// not a memory leak because we copied the pointer
+	Training_message_queue[Training_message_queue_count].special_message.reset();
 }
 
 /**
@@ -1056,7 +1042,7 @@ void message_training_queue_check()
 
 	for (int i=0; i<Training_message_queue_count; i++) {
 		if (timestamp_elapsed(Training_message_queue[i].timestamp)) {
-			message_training_setup(Training_message_queue[i].num, Training_message_queue[i].length, Training_message_queue[i].special_message);
+			message_training_setup(Training_message_queue[i].num, Training_message_queue[i].length, Training_message_queue[i].special_message.get());
 
 			// remove this message from the queue now.
 			message_training_remove_from_queue(i);

From 9c9a1e23d1266c10b35f104d3e0b592f2ea75562 Mon Sep 17 00:00:00 2001
From: Goober5000 <ipw47@mailbox.org>
Date: Fri, 12 Jun 2026 13:21:16 -0400
Subject: [PATCH 5/5] fix geometry_batcher copy semantics and add move
 operations

clone() allocated fresh vert/radius_list buffers without freeing the
ones already owned, so assigning over a live batcher leaked both.  It
also never copied buffer_offset, and the copy constructor called it
with no member initialization at all -- so every copy-constructed
batcher carried an uninitialized buffer_offset.  Free the existing
buffers in clone(), copy buffer_offset, and delegate the copy
constructor to the default constructor so clone() sees initialized
members (which is also what makes its new frees safe on that path).

Also add noexcept move operations, which the user-declared copy
operations had suppressed, so by-value handling can transfer the
buffers instead of deep-copying them.

None of these defects were reachable from current callers (the static
batch maps mutate entries in place); this closes the latent hazards.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 code/graphics/grbatch.cpp | 42 +++++++++++++++++++++++++++++++++++++++
 code/graphics/grbatch.h   |  8 ++++++--
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/code/graphics/grbatch.cpp b/code/graphics/grbatch.cpp
index 36e2b123243..9711b573ff6 100644
--- a/code/graphics/grbatch.cpp
+++ b/code/graphics/grbatch.cpp
@@ -13,6 +13,8 @@
 #include "graphics/grbatch.h"
 #include "render/3d.h"
 
+#include <utility>
+
 geometry_batcher::~geometry_batcher()
 {
 	if (vert != NULL) {
@@ -124,6 +126,15 @@ void geometry_batcher::clone(const geometry_batcher &geo)
 	n_to_render = geo.n_to_render;
 	n_allocated = geo.n_allocated;
 	use_radius = geo.use_radius;
+	buffer_offset = geo.buffer_offset;
+
+	// free any buffers we already own, so that assignment doesn't leak them
+	if (vert != nullptr) {
+		vm_free(vert);
+	}
+	if (radius_list != nullptr) {
+		vm_free(radius_list);
+	}
 
 	if (n_allocated > 0) {
 		vert = (vertex *) vm_malloc( sizeof(vertex) * n_allocated );
@@ -146,6 +157,37 @@ geometry_batcher& geometry_batcher::operator=(const geometry_batcher &geo)
 	return *this;
 }
 
+geometry_batcher::geometry_batcher(geometry_batcher &&other) noexcept
+	: n_to_render(std::exchange(other.n_to_render, 0)),
+	  n_allocated(std::exchange(other.n_allocated, 0)),
+	  vert(std::exchange(other.vert, nullptr)),
+	  use_radius(std::exchange(other.use_radius, true)),
+	  radius_list(std::exchange(other.radius_list, nullptr)),
+	  buffer_offset(std::exchange(other.buffer_offset, -1))
+{
+}
+
+geometry_batcher& geometry_batcher::operator=(geometry_batcher &&other) noexcept
+{
+	if (this != &other) {
+		if (vert != nullptr) {
+			vm_free(vert);
+		}
+		if (radius_list != nullptr) {
+			vm_free(radius_list);
+		}
+
+		n_to_render = std::exchange(other.n_to_render, 0);
+		n_allocated = std::exchange(other.n_allocated, 0);
+		vert = std::exchange(other.vert, nullptr);
+		use_radius = std::exchange(other.use_radius, true);
+		radius_list = std::exchange(other.radius_list, nullptr);
+		buffer_offset = std::exchange(other.buffer_offset, -1);
+	}
+
+	return *this;
+}
+
 /*
 0----1
 |\   |
diff --git a/code/graphics/grbatch.h b/code/graphics/grbatch.h
index 8a8a4d0734c..b6ea1af39be 100644
--- a/code/graphics/grbatch.h
+++ b/code/graphics/grbatch.h
@@ -32,8 +32,12 @@ class geometry_batcher
 	geometry_batcher(): n_to_render(0), n_allocated(0), vert(NULL), use_radius(true), radius_list(NULL), buffer_offset(-1) {}
 	~geometry_batcher();
 
-    geometry_batcher(const geometry_batcher &geo) { clone(geo); }
-    geometry_batcher& operator=(const geometry_batcher &geo);
+	// delegate to the default constructor so that clone() sees initialized members
+	geometry_batcher(const geometry_batcher &geo) : geometry_batcher() { clone(geo); }
+	geometry_batcher& operator=(const geometry_batcher &geo);
+
+	geometry_batcher(geometry_batcher &&other) noexcept;
+	geometry_batcher& operator=(geometry_batcher &&other) noexcept;
 
 	// initial memory space needed
 	// NOTE: This MUST be called BEFORE calling any of the draw_*() functions!!