Merge pull request #56125 from madmiraal/fix-55850

platform/linuxbsd/joypad_linux.cpp

@@ -65,6 +65,7 @@ void JoypadLinux::Joypad::reset() {
 		abs_map[i] = -1;
 		curr_axis[i] = 0;
 	}
+	events.clear();
 }
 
 JoypadLinux::JoypadLinux(Input *in) {
@@ -84,23 +85,26 @@ JoypadLinux::JoypadLinux(Input *in) {
 	print_verbose("JoypadLinux: udev disabled, parsing /dev/input to detect joypads.");
 #endif
 	input = in;
-	joy_thread.start(joy_thread_func, this);
+	monitor_joypads_thread.start(monitor_joypads_thread_func, this);
+	joypad_events_thread.start(joypad_events_thread_func, this);
 }
 
 JoypadLinux::~JoypadLinux() {
-	exit_monitor.set();
-	joy_thread.wait_to_finish();
-	close_joypad();
+	monitor_joypads_exit.set();
+	joypad_events_exit.set();
+	monitor_joypads_thread.wait_to_finish();
+	joypad_events_thread.wait_to_finish();
+	close_joypads();
 }
 
-void JoypadLinux::joy_thread_func(void *p_user) {
+void JoypadLinux::monitor_joypads_thread_func(void *p_user) {
 	if (p_user) {
 		JoypadLinux *joy = static_cast<JoypadLinux *>(p_user);
-		joy->run_joypad_thread();
+		joy->monitor_joypads_thread_run();
 	}
 }
 
-void JoypadLinux::run_joypad_thread() {
+void JoypadLinux::monitor_joypads_thread_run() {
 #ifdef UDEV_ENABLED
 	if (use_udev) {
 		udev *_udev = udev_new();
@@ -140,7 +144,6 @@ void JoypadLinux::enumerate_joypads(udev *p_udev) {
 		if (devnode) {
 			String devnode_str = devnode;
 			if (devnode_str.find(ignore_str) == -1) {
-				MutexLock lock(joy_mutex);
 				open_joypad(devnode);
 			}
 		}
@@ -156,7 +159,7 @@ void JoypadLinux::monitor_joypads(udev *p_udev) {
 	udev_monitor_enable_receiving(mon);
 	int fd = udev_monitor_get_fd(mon);
 
-	while (!exit_monitor.is_set()) {
+	while (!monitor_joypads_exit.is_set()) {
 		fd_set fds;
 		struct timeval tv;
 		int ret;
@@ -175,7 +178,6 @@ void JoypadLinux::monitor_joypads(udev *p_udev) {
 			dev = udev_monitor_receive_device(mon);
 
 			if (dev && udev_device_get_devnode(dev) != nullptr) {
-				MutexLock lock(joy_mutex);
 				String action = udev_device_get_action(dev);
 				const char *devnode = udev_device_get_devnode(dev);
 				if (devnode) {
@@ -184,11 +186,10 @@ void JoypadLinux::monitor_joypads(udev *p_udev) {
 						if (action == "add") {
 							open_joypad(devnode);
 						} else if (String(action) == "remove") {
-							close_joypad(get_joy_from_path(devnode));
+							close_joypad(devnode);
 						}
 					}
 				}
-
 				udev_device_unref(dev);
 			}
 		}
@@ -199,59 +200,54 @@ void JoypadLinux::monitor_joypads(udev *p_udev) {
 #endif
 
 void JoypadLinux::monitor_joypads() {
-	while (!exit_monitor.is_set()) {
-		{
-			MutexLock lock(joy_mutex);
-
-			DIR *input_directory;
-			input_directory = opendir("/dev/input");
-			if (input_directory) {
-				struct dirent *current;
-				char fname[64];
-
-				while ((current = readdir(input_directory)) != nullptr) {
-					if (strncmp(current->d_name, "event", 5) != 0) {
-						continue;
-					}
-					sprintf(fname, "/dev/input/%.*s", 16, current->d_name);
-					if (attached_devices.find(fname) == -1) {
-						open_joypad(fname);
-					}
+	while (!monitor_joypads_exit.is_set()) {
+		DIR *input_directory;
+		input_directory = opendir("/dev/input");
+		if (input_directory) {
+			struct dirent *current;
+			char fname[64];
+
+			while ((current = readdir(input_directory)) != nullptr) {
+				if (strncmp(current->d_name, "event", 5) != 0) {
+					continue;
+				}
+				sprintf(fname, "/dev/input/%.*s", 16, current->d_name);
+				if (attached_devices.find(fname) == -1) {
+					open_joypad(fname);
 				}
 			}
-			closedir(input_directory);
 		}
-		usleep(1000000); // 1s
+		closedir(input_directory);
 	}
+	usleep(1000000); // 1s
 }
 
-int JoypadLinux::get_joy_from_path(String p_path) const {
+void JoypadLinux::close_joypads() {
 	for (int i = 0; i < JOYPADS_MAX; i++) {
-		if (joypads[i].devpath == p_path) {
-			return i;
-		}
+		MutexLock lock(joypads_mutex[i]);
+		Joypad &joypad = joypads[i];
+		close_joypad(joypad, i);
 	}
-	return -2;
 }
 
-void JoypadLinux::close_joypad(int p_id) {
-	if (p_id == -1) {
-		for (int i = 0; i < JOYPADS_MAX; i++) {
-			close_joypad(i);
+void JoypadLinux::close_joypad(const char *p_devpath) {
+	for (int i = 0; i < JOYPADS_MAX; i++) {
+		MutexLock lock(joypads_mutex[i]);
+		Joypad &joypad = joypads[i];
+		if (joypads[i].devpath == p_devpath) {
+			close_joypad(joypad, i);
 		}
-		return;
-	} else if (p_id < 0) {
-		return;
 	}
+}
 
-	Joypad &joy = joypads[p_id];
-
-	if (joy.fd != -1) {
-		close(joy.fd);
-		joy.fd = -1;
-		attached_devices.remove_at(attached_devices.find(joy.devpath));
+void JoypadLinux::close_joypad(Joypad &p_joypad, int p_id) {
+	if (p_joypad.fd != -1) {
+		close(p_joypad.fd);
+		p_joypad.fd = -1;
+		attached_devices.erase(p_joypad.devpath);
 		input->joy_connection_changed(p_id, false, "");
 	}
+	p_joypad.events.clear();
 }
 
 static String _hex_str(uint8_t p_byte) {
@@ -265,27 +261,25 @@ static String _hex_str(uint8_t p_byte) {
 	return ret;
 }
 
-void JoypadLinux::setup_joypad_properties(int p_id) {
-	Joypad *joy = &joypads[p_id];
-
+void JoypadLinux::setup_joypad_properties(Joypad &p_joypad) {
 	unsigned long keybit[NBITS(KEY_MAX)] = { 0 };
 	unsigned long absbit[NBITS(ABS_MAX)] = { 0 };
 
 	int num_buttons = 0;
 	int num_axes = 0;
 
-	if ((ioctl(joy->fd, EVIOCGBIT(EV_KEY, sizeof(keybit)), keybit) < 0) ||
-			(ioctl(joy->fd, EVIOCGBIT(EV_ABS, sizeof(absbit)), absbit) < 0)) {
+	if ((ioctl(p_joypad.fd, EVIOCGBIT(EV_KEY, sizeof(keybit)), keybit) < 0) ||
+			(ioctl(p_joypad.fd, EVIOCGBIT(EV_ABS, sizeof(absbit)), absbit) < 0)) {
 		return;
 	}
 	for (int i = BTN_JOYSTICK; i < KEY_MAX; ++i) {
 		if (test_bit(i, keybit)) {
-			joy->key_map[i] = num_buttons++;
+			p_joypad.key_map[i] = num_buttons++;
 		}
 	}
 	for (int i = BTN_MISC; i < BTN_JOYSTICK; ++i) {
 		if (test_bit(i, keybit)) {
-			joy->key_map[i] = num_buttons++;
+			p_joypad.key_map[i] = num_buttons++;
 		}
 	}
 	for (int i = 0; i < ABS_MISC; ++i) {
@@ -295,21 +289,21 @@ void JoypadLinux::setup_joypad_properties(int p_id) {
 			continue;
 		}
 		if (test_bit(i, absbit)) {
-			joy->abs_map[i] = num_axes++;
-			joy->abs_info[i] = memnew(input_absinfo);
-			if (ioctl(joy->fd, EVIOCGABS(i), joy->abs_info[i]) < 0) {
-				memdelete(joy->abs_info[i]);
-				joy->abs_info[i] = nullptr;
+			p_joypad.abs_map[i] = num_axes++;
+			p_joypad.abs_info[i] = memnew(input_absinfo);
+			if (ioctl(p_joypad.fd, EVIOCGABS(i), p_joypad.abs_info[i]) < 0) {
+				memdelete(p_joypad.abs_info[i]);
+				p_joypad.abs_info[i] = nullptr;
 			}
 		}
 	}
 
-	joy->force_feedback = false;
-	joy->ff_effect_timestamp = 0;
+	p_joypad.force_feedback = false;
+	p_joypad.ff_effect_timestamp = 0;
 	unsigned long ffbit[NBITS(FF_CNT)];
-	if (ioctl(joy->fd, EVIOCGBIT(EV_FF, sizeof(ffbit)), ffbit) != -1) {
+	if (ioctl(p_joypad.fd, EVIOCGBIT(EV_FF, sizeof(ffbit)), ffbit) != -1) {
 		if (test_bit(FF_RUMBLE, ffbit)) {
-			joy->force_feedback = true;
+			p_joypad.force_feedback = true;
 		}
 	}
 }
@@ -353,12 +347,12 @@ void JoypadLinux::open_joypad(const char *p_path) {
 			return;
 		}
 
-		joypads[joy_num].reset();
-
-		Joypad &joy = joypads[joy_num];
-		joy.fd = fd;
-		joy.devpath = String(p_path);
-		setup_joypad_properties(joy_num);
+		MutexLock lock(joypads_mutex[joy_num]);
+		Joypad &joypad = joypads[joy_num];
+		joypad.reset();
+		joypad.fd = fd;
+		joypad.devpath = String(p_path);
+		setup_joypad_properties(joypad);
 		sprintf(uid, "%04x%04x", BSWAP16(inpid.bustype), 0);
 		if (inpid.vendor && inpid.product && inpid.version) {
 			uint16_t vendor = BSWAP16(inpid.vendor);
@@ -379,13 +373,12 @@ void JoypadLinux::open_joypad(const char *p_path) {
 	}
 }
 
-void JoypadLinux::joypad_vibration_start(int p_id, float p_weak_magnitude, float p_strong_magnitude, float p_duration, uint64_t p_timestamp) {
-	Joypad &joy = joypads[p_id];
-	if (!joy.force_feedback || joy.fd == -1 || p_weak_magnitude < 0.f || p_weak_magnitude > 1.f || p_strong_magnitude < 0.f || p_strong_magnitude > 1.f) {
+void JoypadLinux::joypad_vibration_start(Joypad &p_joypad, float p_weak_magnitude, float p_strong_magnitude, float p_duration, uint64_t p_timestamp) {
+	if (!p_joypad.force_feedback || p_joypad.fd == -1 || p_weak_magnitude < 0.f || p_weak_magnitude > 1.f || p_strong_magnitude < 0.f || p_strong_magnitude > 1.f) {
 		return;
 	}
-	if (joy.ff_effect_id != -1) {
-		joypad_vibration_stop(p_id, p_timestamp);
+	if (p_joypad.ff_effect_id != -1) {
+		joypad_vibration_stop(p_joypad, p_timestamp);
 	}
 
 	struct ff_effect effect;
@@ -396,7 +389,7 @@ void JoypadLinux::joypad_vibration_start(int p_id, float p_weak_magnitude, float
 	effect.replay.length = floor(p_duration * 1000);
 	effect.replay.delay = 0;
 
-	if (ioctl(joy.fd, EVIOCSFF, &effect) < 0) {
+	if (ioctl(p_joypad.fd, EVIOCSFF, &effect) < 0) {
 		return;
 	}
 
@@ -404,26 +397,25 @@ void JoypadLinux::joypad_vibration_start(int p_id, float p_weak_magnitude, float
 	play.type = EV_FF;
 	play.code = effect.id;
 	play.value = 1;
-	if (write(joy.fd, (const void *)&play, sizeof(play)) == -1) {
+	if (write(p_joypad.fd, (const void *)&play, sizeof(play)) == -1) {
 		print_verbose("Couldn't write to Joypad device.");
 	}
 
-	joy.ff_effect_id = effect.id;
-	joy.ff_effect_timestamp = p_timestamp;
+	p_joypad.ff_effect_id = effect.id;
+	p_joypad.ff_effect_timestamp = p_timestamp;
 }
 
-void JoypadLinux::joypad_vibration_stop(int p_id, uint64_t p_timestamp) {
-	Joypad &joy = joypads[p_id];
-	if (!joy.force_feedback || joy.fd == -1 || joy.ff_effect_id == -1) {
+void JoypadLinux::joypad_vibration_stop(Joypad &p_joypad, uint64_t p_timestamp) {
+	if (!p_joypad.force_feedback || p_joypad.fd == -1 || p_joypad.ff_effect_id == -1) {
 		return;
 	}
 
-	if (ioctl(joy.fd, EVIOCRMFF, joy.ff_effect_id) < 0) {
+	if (ioctl(p_joypad.fd, EVIOCRMFF, p_joypad.ff_effect_id) < 0) {
 		return;
 	}
 
-	joy.ff_effect_id = -1;
-	joy.ff_effect_timestamp = p_timestamp;
+	p_joypad.ff_effect_id = -1;
+	p_joypad.ff_effect_timestamp = p_timestamp;
 }
 
 float JoypadLinux::axis_correct(const input_absinfo *p_abs, int p_value) const {
@@ -433,104 +425,124 @@ float JoypadLinux::axis_correct(const input_absinfo *p_abs, int p_value) const {
 	return 2.0f * (p_value - min) / (max - min) - 1.0f;
 }
 
-void JoypadLinux::process_joypads() {
-	if (joy_mutex.try_lock() != OK) {
-		return;
+void JoypadLinux::joypad_events_thread_func(void *p_user) {
+	if (p_user) {
+		JoypadLinux *joy = (JoypadLinux *)p_user;
+		joy->joypad_events_thread_run();
 	}
+}
+
+void JoypadLinux::joypad_events_thread_run() {
+	while (!joypad_events_exit.is_set()) {
+		bool no_events = true;
+		for (int i = 0; i < JOYPADS_MAX; i++) {
+			MutexLock lock(joypads_mutex[i]);
+			Joypad &joypad = joypads[i];
+			if (joypad.fd == -1) {
+				continue;
+			}
+			input_event event;
+			while (read(joypad.fd, &event, sizeof(event)) > 0) {
+				no_events = false;
+				JoypadEvent joypad_event;
+				joypad_event.type = event.type;
+				joypad_event.code = event.code;
+				joypad_event.value = event.value;
+				joypad.events.push_back(joypad_event);
+			}
+			if (errno != EAGAIN) {
+				close_joypad(joypad, i);
+			}
+		}
+		if (no_events) {
+			usleep(10000); // 10ms
+		}
+	}
+}
+
+void JoypadLinux::process_joypads() {
 	for (int i = 0; i < JOYPADS_MAX; i++) {
-		if (joypads[i].fd == -1) {
+		MutexLock lock(joypads_mutex[i]);
+		Joypad &joypad = joypads[i];
+		if (joypad.fd == -1) {
 			continue;
 		}
+		for (uint32_t j = 0; j < joypad.events.size(); j++) {
+			const JoypadEvent &joypad_event = joypad.events[j];
+			// joypad_event may be tainted and out of MAX_KEY range, which will cause
+			// joypad.key_map[joypad_event.code] to crash
+			if (joypad_event.code >= MAX_KEY) {
+				return;
+			}
 
-		input_event events[32];
-		Joypad *joy = &joypads[i];
-
-		int len;
-
-		while ((len = read(joy->fd, events, (sizeof events))) > 0) {
-			len /= sizeof(events[0]);
-			for (int j = 0; j < len; j++) {
-				input_event &ev = events[j];
-
-				// ev may be tainted and out of MAX_KEY range, which will cause
-				// joy->key_map[ev.code] to crash
-				if (ev.code >= MAX_KEY) {
-					return;
-				}
-
-				switch (ev.type) {
-					case EV_KEY:
-						input->joy_button(i, (JoyButton)joy->key_map[ev.code], ev.value);
-						break;
-
-					case EV_ABS:
-
-						switch (ev.code) {
-							case ABS_HAT0X:
-								if (ev.value != 0) {
-									if (ev.value < 0) {
-										joy->dpad = (HatMask)((joy->dpad | HatMask::LEFT) & ~HatMask::RIGHT);
-									} else {
-										joy->dpad = (HatMask)((joy->dpad | HatMask::RIGHT) & ~HatMask::LEFT);
-									}
+			switch (joypad_event.type) {
+				case EV_KEY:
+					input->joy_button(i, (JoyButton)joypad.key_map[joypad_event.code], joypad_event.value);
+					break;
+
+				case EV_ABS:
+					switch (joypad_event.code) {
+						case ABS_HAT0X:
+							if (joypad_event.value != 0) {
+								if (joypad_event.value < 0) {
+									joypad.dpad = (HatMask)((joypad.dpad | HatMask::LEFT) & ~HatMask::RIGHT);
 								} else {
-									joy->dpad &= ~(HatMask::LEFT | HatMask::RIGHT);
+									joypad.dpad = (HatMask)((joypad.dpad | HatMask::RIGHT) & ~HatMask::LEFT);
 								}
-
-								input->joy_hat(i, (HatMask)joy->dpad);
-								break;
-
-							case ABS_HAT0Y:
-								if (ev.value != 0) {
-									if (ev.value < 0) {
-										joy->dpad = (HatMask)((joy->dpad | HatMask::UP) & ~HatMask::DOWN);
-									} else {
-										joy->dpad = (HatMask)((joy->dpad | HatMask::DOWN) & ~HatMask::UP);
-									}
+							} else {
+								joypad.dpad &= ~(HatMask::LEFT | HatMask::RIGHT);
+							}
+							input->joy_hat(i, (HatMask)joypad.dpad);
+							break;
+
+						case ABS_HAT0Y:
+							if (joypad_event.value != 0) {
+								if (joypad_event.value < 0) {
+									joypad.dpad = (HatMask)((joypad.dpad | HatMask::UP) & ~HatMask::DOWN);
 								} else {
-									joy->dpad &= ~(HatMask::UP | HatMask::DOWN);
+									joypad.dpad = (HatMask)((joypad.dpad | HatMask::DOWN) & ~HatMask::UP);
 								}
-
-								input->joy_hat(i, (HatMask)joy->dpad);
-								break;
-
-							default:
-								if (ev.code >= MAX_ABS) {
-									return;
-								}
-								if (joy->abs_map[ev.code] != -1 && joy->abs_info[ev.code]) {
-									float value = axis_correct(joy->abs_info[ev.code], ev.value);
-									joy->curr_axis[joy->abs_map[ev.code]] = value;
-								}
-								break;
-						}
-						break;
-				}
+							} else {
+								joypad.dpad &= ~(HatMask::UP | HatMask::DOWN);
+							}
+							input->joy_hat(i, (HatMask)joypad.dpad);
+							break;
+
+						default:
+							if (joypad_event.code >= MAX_ABS) {
+								return;
+							}
+							if (joypad.abs_map[joypad_event.code] != -1 && joypad.abs_info[joypad_event.code]) {
+								float value = axis_correct(joypad.abs_info[joypad_event.code], joypad_event.value);
+								joypad.curr_axis[joypad.abs_map[joypad_event.code]] = value;
+							}
+							break;
+					}
+					break;
 			}
 		}
+		joypad.events.clear();
+
 		for (int j = 0; j < MAX_ABS; j++) {
-			int index = joy->abs_map[j];
+			int index = joypad.abs_map[j];
 			if (index != -1) {
-				input->joy_axis(i, (JoyAxis)index, joy->curr_axis[index]);
+				input->joy_axis(i, (JoyAxis)index, joypad.curr_axis[index]);
 			}
 		}
-		if (len == 0 || (len < 0 && errno != EAGAIN)) {
-			close_joypad(i);
-		}
 
-		if (joy->force_feedback) {
+		if (joypad.force_feedback) {
 			uint64_t timestamp = input->get_joy_vibration_timestamp(i);
-			if (timestamp > joy->ff_effect_timestamp) {
+			if (timestamp > joypad.ff_effect_timestamp) {
 				Vector2 strength = input->get_joy_vibration_strength(i);
 				float duration = input->get_joy_vibration_duration(i);
 				if (strength.x == 0 && strength.y == 0) {
-					joypad_vibration_stop(i, timestamp);
+					joypad_vibration_stop(joypad, timestamp);
 				} else {
-					joypad_vibration_start(i, strength.x, strength.y, duration, timestamp);
+					joypad_vibration_start(joypad, strength.x, strength.y, duration, timestamp);
 				}
 			}
 		}
 	}
-	joy_mutex.unlock();
 }
-#endif
+
+#endif // JOYDEV_ENABLED

platform/linuxbsd/joypad_linux.h

@@ -35,6 +35,7 @@
 #include "core/input/input.h"
 #include "core/os/mutex.h"
 #include "core/os/thread.h"
+#include "core/templates/local_vector.h"
 
 struct input_absinfo;
 
@@ -51,6 +52,12 @@ private:
 		MAX_KEY = 767, // Hack because <linux/input.h> can't be included here
 	};
 
+	struct JoypadEvent {
+		uint16_t type;
+		uint16_t code;
+		int32_t value;
+	};
+
 	struct Joypad {
 		float curr_axis[MAX_ABS];
 		int key_map[MAX_KEY];
@@ -65,6 +72,8 @@ private:
 		int ff_effect_id = 0;
 		uint64_t ff_effect_timestamp = 0;
 
+		LocalVector<JoypadEvent> events;
+
 		~Joypad();
 		void reset();
 	};
@@ -72,29 +81,39 @@ private:
 #ifdef UDEV_ENABLED
 	bool use_udev = false;
 #endif
-	SafeFlag exit_monitor;
-	Mutex joy_mutex;
-	Thread joy_thread;
 	Input *input = nullptr;
+
+	SafeFlag monitor_joypads_exit;
+	SafeFlag joypad_events_exit;
+	Thread monitor_joypads_thread;
+	Thread joypad_events_thread;
+
 	Joypad joypads[JOYPADS_MAX];
+	Mutex joypads_mutex[JOYPADS_MAX];
+
 	Vector<String> attached_devices;
 
-	static void joy_thread_func(void *p_user);
+	static void monitor_joypads_thread_func(void *p_user);
+	void monitor_joypads_thread_run();
+
+	void open_joypad(const char *p_path);
+	void setup_joypad_properties(Joypad &p_joypad);
 
-	int get_joy_from_path(String p_path) const;
+	void close_joypads();
+	void close_joypad(const char *p_devpath);
+	void close_joypad(Joypad &p_joypad, int p_id);
 
-	void setup_joypad_properties(int p_id);
-	void close_joypad(int p_id = -1);
 #ifdef UDEV_ENABLED
 	void enumerate_joypads(struct udev *p_udev);
 	void monitor_joypads(struct udev *p_udev);
 #endif
 	void monitor_joypads();
-	void run_joypad_thread();
-	void open_joypad(const char *p_path);
 
-	void joypad_vibration_start(int p_id, float p_weak_magnitude, float p_strong_magnitude, float p_duration, uint64_t p_timestamp);
-	void joypad_vibration_stop(int p_id, uint64_t p_timestamp);
+	void joypad_vibration_start(Joypad &p_joypad, float p_weak_magnitude, float p_strong_magnitude, float p_duration, uint64_t p_timestamp);
+	void joypad_vibration_stop(Joypad &p_joypad, uint64_t p_timestamp);
+
+	static void joypad_events_thread_func(void *p_user);
+	void joypad_events_thread_run();
 
 	float axis_correct(const input_absinfo *p_abs, int p_value) const;
 };