Switched AnimatedTexture to a readers-writers lock, solves a race condition and fixes #20221

4 files changed, 30 insertions, 17 deletions

diff --git a/core/os/rw_lock.h b/core/os/rw_lock.h
index 9053794c83..3e53300c9f 100644
--- a/core/os/rw_lock.h
+++ b/core/os/rw_lock.h
@@ -56,8 +56,10 @@ class RWLockRead {
 	RWLock *lock;
 
 public:
-	RWLockRead(RWLock *p_lock) {
-		lock = p_lock;
+	RWLockRead(const RWLock *p_lock) {
+		if (p_lock) {
+			lock = const_cast<RWLock *>(p_lock);
+		}
 		if (lock) lock->read_lock();
 	}
 	~RWLockRead() {
diff --git a/scene/3d/remote_transform.cpp b/scene/3d/remote_transform.cpp
index 2156e24cd0..c12e49fb47 100644
--- a/scene/3d/remote_transform.cpp
+++ b/scene/3d/remote_transform.cpp
@@ -124,8 +124,10 @@ void RemoteTransform::_notification(int p_what) {
 void RemoteTransform::set_remote_node(const NodePath &p_remote_node) {
 
 	remote_node = p_remote_node;
-	if (is_inside_tree())
+	if (is_inside_tree()) {
 		_update_cache();
+		_update_remote();
+	}
 
 	update_configuration_warning();
 }
diff --git a/scene/resources/texture.cpp b/scene/resources/texture.cpp
index d8efbeba17..41f3ee1fce 100644
--- a/scene/resources/texture.cpp
+++ b/scene/resources/texture.cpp
@@ -1666,7 +1666,7 @@ ProxyTexture::~ProxyTexture() {
 
 void AnimatedTexture::_update_proxy() {
 
-	_THREAD_SAFE_METHOD_
+	RWLockRead r(rw_lock);
 
 	float delta;
 	if (prev_ticks == 0) {
@@ -1712,7 +1712,7 @@ void AnimatedTexture::_update_proxy() {
 void AnimatedTexture::set_frames(int p_frames) {
 	ERR_FAIL_COND(p_frames < 1 || p_frames > MAX_FRAMES);
 
-	_THREAD_SAFE_METHOD_
+	RWLockWrite r(rw_lock);
 
 	frame_count = p_frames;
 }
@@ -1723,14 +1723,14 @@ int AnimatedTexture::get_frames() const {
 void AnimatedTexture::set_frame_texture(int p_frame, const Ref<Texture> &p_texture) {
 	ERR_FAIL_INDEX(p_frame, MAX_FRAMES);
 
-	_THREAD_SAFE_METHOD_
+	RWLockWrite w(rw_lock);
 
 	frames[p_frame].texture = p_texture;
 }
 Ref<Texture> AnimatedTexture::get_frame_texture(int p_frame) const {
 	ERR_FAIL_INDEX_V(p_frame, MAX_FRAMES, Ref<Texture>());
 
-	_THREAD_SAFE_METHOD_
+	RWLockRead r(rw_lock);
 
 	return frames[p_frame].texture;
 }
@@ -1738,14 +1738,14 @@ Ref<Texture> AnimatedTexture::get_frame_texture(int p_frame) const {
 void AnimatedTexture::set_frame_delay(int p_frame, float p_delay_sec) {
 	ERR_FAIL_INDEX(p_frame, MAX_FRAMES);
 
-	_THREAD_SAFE_METHOD_
+	RWLockRead r(rw_lock);
 
 	frames[p_frame].delay_sec = p_delay_sec;
 }
 float AnimatedTexture::get_frame_delay(int p_frame) const {
 	ERR_FAIL_INDEX_V(p_frame, MAX_FRAMES, 0);
 
-	_THREAD_SAFE_METHOD_
+	RWLockRead r(rw_lock);
 
 	return frames[p_frame].delay_sec;
 }
@@ -1760,8 +1760,7 @@ float AnimatedTexture::get_fps() const {
 }
 
 int AnimatedTexture::get_width() const {
-
-	_THREAD_SAFE_METHOD_
+	RWLockRead r(rw_lock);
 
 	if (!frames[current_frame].texture.is_valid()) {
 		return 1;
@@ -1770,8 +1769,7 @@ int AnimatedTexture::get_width() const {
 	return frames[current_frame].texture->get_width();
 }
 int AnimatedTexture::get_height() const {
-
-	_THREAD_SAFE_METHOD_
+	RWLockRead r(rw_lock);
 
 	if (!frames[current_frame].texture.is_valid()) {
 		return 1;
@@ -1785,7 +1783,7 @@ RID AnimatedTexture::get_rid() const {
 
 bool AnimatedTexture::has_alpha() const {
 
-	_THREAD_SAFE_METHOD_
+	RWLockRead r(rw_lock);
 
 	if (!frames[current_frame].texture.is_valid()) {
 		return false;
@@ -1796,7 +1794,7 @@ bool AnimatedTexture::has_alpha() const {
 
 Ref<Image> AnimatedTexture::get_data() const {
 
-	_THREAD_SAFE_METHOD_
+	RWLockRead r(rw_lock);
 
 	if (!frames[current_frame].texture.is_valid()) {
 		return Ref<Image>();
@@ -1809,7 +1807,7 @@ void AnimatedTexture::set_flags(uint32_t p_flags) {
 }
 uint32_t AnimatedTexture::get_flags() const {
 
-	_THREAD_SAFE_METHOD_
+	RWLockRead r(rw_lock);
 
 	if (!frames[current_frame].texture.is_valid()) {
 		return 0;
@@ -1862,10 +1860,19 @@ AnimatedTexture::AnimatedTexture() {
 	prev_ticks = 0;
 	current_frame = 0;
 	VisualServer::get_singleton()->connect("frame_pre_draw", this, "_update_proxy");
+
+#ifndef NO_THREADS
+	rw_lock = RWLock::create();
+#else
+	rw_lock = NULL;
+#endif
 }
 
 AnimatedTexture::~AnimatedTexture() {
 	VS::get_singleton()->free(proxy);
+	if (rw_lock) {
+		memdelete(rw_lock);
+	}
 }
 ///////////////////////////////
 
diff --git a/scene/resources/texture.h b/scene/resources/texture.h
index c1331fb3fe..b02cbb8fa8 100644
--- a/scene/resources/texture.h
+++ b/scene/resources/texture.h
@@ -34,6 +34,7 @@
 #include "curve.h"
 #include "io/resource_loader.h"
 #include "os/mutex.h"
+#include "os/rw_lock.h"
 #include "os/thread_safe.h"
 #include "rect2.h"
 #include "resource.h"
@@ -609,7 +610,8 @@ public:
 class AnimatedTexture : public Texture {
 	GDCLASS(AnimatedTexture, Texture)
 
-	_THREAD_SAFE_CLASS_
+	//use readers writers lock for this, since its far more times read than written to
+	RWLock *rw_lock;
 
 private:
 	enum {