Skip to content

Commit

Permalink
Add RIDReferences
Browse files Browse the repository at this point in the history
Storing RIDs scene side can lead to dangling RIDs if a RID is freed elsewhere.

RIDReference allows scene side code to reference a RID indirectly, which will automatically be set to NULL when the object is freed, and thus help prevent dangling RIDs.
  • Loading branch information
lawnjelly committed May 29, 2023
1 parent 8a417f3 commit 28b6e6b
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 8 deletions.
2 changes: 2 additions & 0 deletions scene/3d/collision_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "mesh_instance.h"
#include "scene/scene_string_names.h"
#include "servers/physics_server.h"
#include "servers/rid_reference.h"

void CollisionObject::_notification(int p_what) {
switch (p_what) {
Expand Down Expand Up @@ -587,5 +588,6 @@ CollisionObject::CollisionObject() {
}

CollisionObject::~CollisionObject() {
RIDReferences::notify_free_RID(RIDReferences::SERVER_PHYSICS, rid);
PhysicsServer::get_singleton()->free(rid);
}
15 changes: 8 additions & 7 deletions scene/3d/physics_body.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1077,17 +1077,18 @@ Vector3 KinematicBody::_move_and_slide_internal(const Vector3 &p_linear_velocity
float delta = Engine::get_singleton()->is_in_physics_frame() ? get_physics_process_delta_time() : get_process_delta_time();

Vector3 current_floor_velocity = floor_velocity;

if (on_floor && on_floor_body.is_valid()) {
// This approach makes sure there is less delay between the actual body velocity and the one we saved.
PhysicsDirectBodyState *bs = PhysicsServer::get_singleton()->body_get_direct_state(on_floor_body);
PhysicsDirectBodyState *bs = PhysicsServer::get_singleton()->body_get_direct_state(on_floor_body.get());
if (bs) {
Transform gt = get_global_transform();
Vector3 local_position = gt.origin - bs->get_transform().origin;
current_floor_velocity = bs->get_velocity_at_local_position(local_position);
} else {
// Body is removed or destroyed, invalidate floor.
current_floor_velocity = Vector3();
on_floor_body = RID();
on_floor_body.set(RID());
}
}

Expand All @@ -1101,14 +1102,14 @@ Vector3 KinematicBody::_move_and_slide_internal(const Vector3 &p_linear_velocity
if (current_floor_velocity != Vector3() && on_floor_body.is_valid()) {
Collision floor_collision;
Set<RID> exclude;
exclude.insert(on_floor_body);
exclude.insert(on_floor_body.get());
if (move_and_collide(current_floor_velocity * delta, p_infinite_inertia, floor_collision, true, false, false, exclude)) {
colliders.push_back(floor_collision);
_set_collision_direction(floor_collision, up_direction, p_floor_max_angle);
}
}

on_floor_body = RID();
on_floor_body.set(RID());
Vector3 motion = body_velocity * delta;

// No sliding on first attempt to keep floor motion stable when possible,
Expand Down Expand Up @@ -1186,7 +1187,7 @@ Vector3 KinematicBody::_move_and_slide_internal(const Vector3 &p_linear_velocity
if (Math::acos(col.normal.dot(up_direction)) <= p_floor_max_angle + FLOOR_ANGLE_THRESHOLD) {
on_floor = true;
floor_normal = col.normal;
on_floor_body = col.collider_rid;
on_floor_body.set(col.collider_rid);
floor_velocity = col.collider_vel;
if (p_stop_on_slope) {
// move and collide may stray the object a bit because of pre un-stucking,
Expand Down Expand Up @@ -1237,7 +1238,7 @@ void KinematicBody::_set_collision_direction(const Collision &p_collision, const
if (Math::acos(p_collision.normal.dot(p_up_direction)) <= p_floor_max_angle + FLOOR_ANGLE_THRESHOLD) { //floor
on_floor = true;
floor_normal = p_collision.normal;
on_floor_body = p_collision.collider_rid;
on_floor_body.set(p_collision.collider_rid);
floor_velocity = p_collision.collider_vel;
} else if (Math::acos(p_collision.normal.dot(-p_up_direction)) <= p_floor_max_angle + FLOOR_ANGLE_THRESHOLD) { //ceiling
on_ceiling = true;
Expand Down Expand Up @@ -1429,7 +1430,7 @@ void KinematicBody::_notification(int p_what) {

// Reset move_and_slide() data.
on_floor = false;
on_floor_body = RID();
on_floor_body.set(RID());
on_ceiling = false;
on_wall = false;
colliders.clear();
Expand Down
3 changes: 2 additions & 1 deletion scene/3d/physics_body.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "scene/3d/collision_object.h"
#include "scene/resources/physics_material.h"
#include "servers/physics_server.h"
#include "servers/rid_reference.h"
#include "skeleton.h"

class PhysicsBody : public CollisionObject {
Expand Down Expand Up @@ -292,7 +293,7 @@ class KinematicBody : public PhysicsBody {

Vector3 floor_normal;
Vector3 floor_velocity;
RID on_floor_body;
RIDRef<RIDReferences::SERVER_PHYSICS> on_floor_body;
bool on_floor;
bool on_ceiling;
bool on_wall;
Expand Down
73 changes: 73 additions & 0 deletions servers/rid_reference.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/**************************************************************************/
/* rid_reference.cpp */
/**************************************************************************/
/* This file is part of: */
/* GODOT ENGINE */
/* https://godotengine.org */
/**************************************************************************/
/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */
/* */
/* Permission is hereby granted, free of charge, to any person obtaining */
/* a copy of this software and associated documentation files (the */
/* "Software"), to deal in the Software without restriction, including */
/* without limitation the rights to use, copy, modify, merge, publish, */
/* distribute, sublicense, and/or sell copies of the Software, and to */
/* permit persons to whom the Software is furnished to do so, subject to */
/* the following conditions: */
/* */
/* The above copyright notice and this permission notice shall be */
/* included in all copies or substantial portions of the Software. */
/* */
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
/**************************************************************************/

#include "rid_reference.h"

RIDReferences::Server RIDReferences::_servers[SERVER_COUNT];

uint32_t RIDReferences::request_reference(ServerType p_server_type) {
uint32_t id = UINT32_MAX;
MutexLock(_servers[p_server_type]._mutex);
RIDReference *ref = _servers[p_server_type]._pool.request(id);
ref->rid = RID();
return id;
}

void RIDReferences::free_reference(ServerType p_server_type, uint32_t p_id) {
MutexLock(_servers[p_server_type]._mutex);
_servers[p_server_type]._pool.free(p_id);
}

void RIDReferences::set_reference(ServerType p_server_type, uint32_t p_id, RID p_rid) {
MutexLock(_servers[p_server_type]._mutex);
_servers[p_server_type]._pool[p_id].rid = p_rid;
}

RID RIDReferences::get_reference(ServerType p_server_type, uint32_t p_id) {
MutexLock(_servers[p_server_type]._mutex);
return _servers[p_server_type]._pool[p_id].rid;
}

// The important function, when deleting a RID, Null out any references to it,
// so no dangling references will be used.
// Note that this assumes single threaded use from client to server.
// If another thread frees a RID BEFORE the first thread uses it, then a dangling
// RID can still be used in the server.
void RIDReferences::notify_free_RID(ServerType p_server_type, RID p_rid) {
ERR_FAIL_COND(p_rid == RID());
MutexLock(_servers[p_server_type]._mutex);
uint32_t active_size = _servers[p_server_type]._pool.active_size();
for (uint32_t n = 0; n < active_size; n++) {
RIDReference &ref = _servers[p_server_type]._pool.get_active(n);
if (ref.rid == p_rid) {
ref.rid = RID();
}
}
}
100 changes: 100 additions & 0 deletions servers/rid_reference.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/**************************************************************************/
/* rid_reference.h */
/**************************************************************************/
/* This file is part of: */
/* GODOT ENGINE */
/* https://godotengine.org */
/**************************************************************************/
/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */
/* */
/* Permission is hereby granted, free of charge, to any person obtaining */
/* a copy of this software and associated documentation files (the */
/* "Software"), to deal in the Software without restriction, including */
/* without limitation the rights to use, copy, modify, merge, publish, */
/* distribute, sublicense, and/or sell copies of the Software, and to */
/* permit persons to whom the Software is furnished to do so, subject to */
/* the following conditions: */
/* */
/* The above copyright notice and this permission notice shall be */
/* included in all copies or substantial portions of the Software. */
/* */
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
/**************************************************************************/

#ifndef RID_REFERENCES_H
#define RID_REFERENCES_H

#include "core/os/mutex.h"
#include "core/pooled_list.h"
#include "core/rid.h"

class RIDReferences {
public:
enum ServerType {
SERVER_PHYSICS,
SERVER_COUNT,
};
template <RIDReferences::ServerType>
friend class RIDRef;

private:
struct RIDReference {
RID rid;
};
struct Server {
TrackedPooledList<RIDReference> _pool;
Mutex _mutex;
};

static Server _servers[SERVER_COUNT];

static uint32_t request_reference(ServerType p_server_type);
static void free_reference(ServerType p_server_type, uint32_t p_id);
static void set_reference(ServerType p_server_type, uint32_t p_id, RID p_rid);
static RID get_reference(ServerType p_server_type, uint32_t p_id);

public:
static void notify_free_RID(ServerType p_server_type, RID p_rid);
};

template <RIDReferences::ServerType TYPE>
class RIDRef {
uint32_t id = UINT32_MAX;

public:
bool is_valid() const { return id != UINT32_MAX; }
void set(RID p_rid) {
if (p_rid.is_valid()) {
if (!is_valid()) {
id = RIDReferences::request_reference(TYPE);
}
RIDReferences::set_reference(TYPE, id, p_rid);
} else {
if (is_valid()) {
RIDReferences::free_reference(TYPE, id);
id = UINT32_MAX;
}
}
}
RID get() const {
if (is_valid()) {
return RIDReferences::get_reference(TYPE, id);
}
return RID();
}
~RIDRef() {
if (is_valid()) {
RIDReferences::free_reference(TYPE, id);
id = UINT32_MAX;
}
}
};

#endif // RID_REFERENCES_H

0 comments on commit 28b6e6b

Please sign in to comment.