Merge pull request #55764 from tinmanjuggernaut/validate_rid

[3.x] Validate RIDs before freeing

doc/classes/Navigation2DServer.xml

@@ -124,7 +124,8 @@
 			<return type="void" />
 			<argument index="0" name="rid" type="RID" />
 			<description>
-				Destroys the given RID.
+				Destroys an object created by the Navigation2DServer.
+				[b]Note:[/b] See [method VisualServer.free_rid] for details on how to handle RIDs for freed objects.
 			</description>
 		</method>
 		<method name="get_maps" qualifiers="const">

doc/classes/NavigationServer.xml

@@ -123,7 +123,8 @@
 			<return type="void" />
 			<argument index="0" name="rid" type="RID" />
 			<description>
-				Destroys the given RID.
+				Destroys an object created by the NavigationServer.
+				[b]Note:[/b] See [method VisualServer.free_rid] for details on how to handle RIDs for freed objects.
 			</description>
 		</method>
 		<method name="get_maps" qualifiers="const">

doc/classes/Physics2DServer.xml

@@ -654,7 +654,8 @@
 			<return type="void" />
 			<argument index="0" name="rid" type="RID" />
 			<description>
-				Destroys any of the objects created by Physics2DServer. If the [RID] passed is not one of the objects that can be created by Physics2DServer, an error will be sent to the console.
+				Destroys an object created by the Physics2DServer.
+				[b]Note:[/b] See [method VisualServer.free_rid] for details on how to handle RIDs for freed objects.
 			</description>
 		</method>
 		<method name="get_process_info">

doc/classes/PhysicsServer.xml

@@ -628,7 +628,8 @@
 			<return type="void" />
 			<argument index="0" name="rid" type="RID" />
 			<description>
-				Destroys any of the objects created by PhysicsServer. If the [RID] passed is not one of the objects that can be created by PhysicsServer, an error will be sent to the console.
+				Destroys an object created by the PhysicsServer.
+				[b]Note:[/b] See [method VisualServer.free_rid] for details on how to handle RIDs for freed objects.
 			</description>
 		</method>
 		<method name="generic_6dof_joint_get_flag">

doc/classes/VisualServer.xml

@@ -961,7 +961,19 @@
 			<return type="void" />
 			<argument index="0" name="rid" type="RID" />
 			<description>
-				Tries to free an object in the VisualServer.
+				Destroys an object created by the VisualServer. If the [RID] passed is not one created by the server that created it (e.g. VisualServer, PhysicsServer, etc.), an error will be sent to the console.
+				[b]Note:[/b] After freeing the object, the RID now has a reference to invalid memory. It is not safe to use or free an invalid RID. Before using the RID again, make sure to assign it to [code]RID()[/code] or any other valid RID.
+				[codeblock]
+				var r: RID = VisualServer.get_test_cube()
+				VisualServer.free_rid(r)
+				print("ID: ", r.get_id())   # It is not safe to access or free an invalid RID
+				r = RID()                   # Reset the RID so it is safe to use again.
+				print("ID: ", r.get_id())
+
+				# Output:
+				# ID: 157     # Freed RID has invalid data
+				# ID: 0       # RID has been properly reset
+				[/codeblock]
 			</description>
 		</method>
 		<method name="get_render_info">

modules/bullet/bullet_physics_server.cpp

@@ -1480,6 +1480,11 @@ bool BulletPhysicsServer::generic_6dof_joint_get_flag(RID p_joint, Vector3::Axis
 }
 
 void BulletPhysicsServer::free(RID p_rid) {
+	if (!p_rid.is_valid()) {
+		ERR_FAIL_MSG("Invalid RID.");
+		return;
+	}
+
 	if (shape_owner.owns(p_rid)) {
 		ShapeBullet *shape = shape_owner.get(p_rid);
 
@@ -1490,6 +1495,7 @@ void BulletPhysicsServer::free(RID p_rid) {
 
 		shape_owner.free(p_rid);
 		bulletdelete(shape);
+
 	} else if (rigid_body_owner.owns(p_rid)) {
 		RigidBodyBullet *body = rigid_body_owner.get(p_rid);
 
@@ -1532,8 +1538,9 @@ void BulletPhysicsServer::free(RID p_rid) {
 		space_set_active(p_rid, false);
 		space_owner.free(p_rid);
 		bulletdelete(space);
+
 	} else {
-		ERR_FAIL_MSG("Invalid ID.");
+		ERR_FAIL_MSG("Invalid RID.");
 	}
 }
 

modules/navigation/godot_navigation_server.cpp

@@ -553,6 +553,10 @@ COMMAND_4(agent_set_callback, RID, p_agent, Object *, p_receiver, StringName, p_
 }
 
 COMMAND_1(free, RID, p_object) {
+	if (!p_object.is_valid()) {
+		ERR_FAIL_MSG("Invalid RID.");
+		return;
+	}
 	if (map_owner.owns(p_object)) {
 		NavMap *map = map_owner.getornull(p_object);
 
@@ -601,7 +605,7 @@ COMMAND_1(free, RID, p_object) {
 		memdelete(agent);
 
 	} else {
-		ERR_FAIL_COND("Invalid ID.");
+		ERR_FAIL_COND("Invalid RID.");
 	}
 }
 

servers/physics/physics_server_sw.cpp

@@ -1199,6 +1199,11 @@ bool PhysicsServerSW::generic_6dof_joint_get_flag(RID p_joint, Vector3::Axis p_a
 }
 
 void PhysicsServerSW::free(RID p_rid) {
+	if (!p_rid.is_valid()) {
+		ERR_FAIL_MSG("Invalid RID.");
+		return;
+	}
+
 	_update_shapes(); //just in case
 
 	if (shape_owner.owns(p_rid)) {
@@ -1211,6 +1216,7 @@ void PhysicsServerSW::free(RID p_rid) {
 
 		shape_owner.free(p_rid);
 		memdelete(shape);
+
 	} else if (body_owner.owns(p_rid)) {
 		BodySW *body = body_owner.get(p_rid);
 
@@ -1247,6 +1253,7 @@ void PhysicsServerSW::free(RID p_rid) {
 
 		area_owner.free(p_rid);
 		memdelete(area);
+
 	} else if (space_owner.owns(p_rid)) {
 		SpaceSW *space = space_owner.get(p_rid);
 
@@ -1261,6 +1268,7 @@ void PhysicsServerSW::free(RID p_rid) {
 
 		space_owner.free(p_rid);
 		memdelete(space);
+
 	} else if (joint_owner.owns(p_rid)) {
 		JointSW *joint = joint_owner.get(p_rid);
 
@@ -1271,7 +1279,7 @@ void PhysicsServerSW::free(RID p_rid) {
 		memdelete(joint);
 
 	} else {
-		ERR_FAIL_MSG("Invalid ID.");
+		ERR_FAIL_MSG("Invalid RID.");
 	}
 };
 

servers/physics_2d/physics_2d_server_sw.cpp

@@ -1121,6 +1121,11 @@ Physics2DServer::JointType Physics2DServerSW::joint_get_type(RID p_joint) const
 }
 
 void Physics2DServerSW::free(RID p_rid) {
+	if (!p_rid.is_valid()) {
+		ERR_FAIL_MSG("Invalid RID.");
+		return;
+	}
+
 	_update_shapes(); // just in case
 
 	if (shape_owner.owns(p_rid)) {
@@ -1169,6 +1174,7 @@ void Physics2DServerSW::free(RID p_rid) {
 
 		area_owner.free(p_rid);
 		memdelete(area);
+
 	} else if (space_owner.owns(p_rid)) {
 		Space2DSW *space = space_owner.get(p_rid);
 
@@ -1181,6 +1187,7 @@ void Physics2DServerSW::free(RID p_rid) {
 		free(space->get_default_area()->get_self());
 		space_owner.free(p_rid);
 		memdelete(space);
+
 	} else if (joint_owner.owns(p_rid)) {
 		Joint2DSW *joint = joint_owner.get(p_rid);
 
@@ -1188,7 +1195,7 @@ void Physics2DServerSW::free(RID p_rid) {
 		memdelete(joint);
 
 	} else {
-		ERR_FAIL_MSG("Invalid ID.");
+		ERR_FAIL_MSG("Invalid RID.");
 	}
 };
 

servers/visual/visual_server_raster.cpp

@@ -65,6 +65,11 @@ void VisualServerRaster::_draw_margins() {
 /* FREE */
 
 void VisualServerRaster::free(RID p_rid) {
+	if (!p_rid.is_valid()) {
+		ERR_FAIL_MSG("Invalid RID.");
+		return;
+	}
+
 	if (VSG::storage->free(p_rid)) {
 		return;
 	}
@@ -80,6 +85,8 @@ void VisualServerRaster::free(RID p_rid) {
 	if (VSG::scene_render->free(p_rid)) {
 		return;
 	}
+
+	ERR_FAIL_MSG("Invalid RID.");
 }
 
 /* EVENT QUEUING */