* Ignore B_ENTRY_REMOVED node monitor messages.

* When the device watcher/control thread encounteres an error when
  read()ing or ioctl()ing the device, don't just quit the thread and
  leave a stale device add-on hanging there, but instead trigger
  _RemoveDevice() to exit this cleanly. This also takes care of calling
  _RemoveDevice() only from one thread. However, it adds a race condition
  should a mouse or keyboard be unplugged and plugged at the same time.
  I need to think about how to fix that cleanly, although the situation
  may be theoretical only... This fix seems to fix another problem with
  hot-plugging USB mice, before this change, the first mouse entry in
  /dev/input/mouse/usb/ was never gone and I got two entries after unplugging
  and replugging.
* When using BObjectList configured to own the entries - don't delete the
  entries! Also don't call RemoveItem() before still using the item. Took
  me all day to find this one, because the code looked so... correct. :-}
* In _AddDevice() call _RemoveDevice() just for the sake of it. It is really
  important that no device with the same name is published twice. The PS/2
  driver behaves strange in that it publishes device more than once, if
  I understand correctly, until it decides that there is no device.
* Only StartMonitoringDevice() /after/ having performed the initial device
  scan! Or else we may get ourselves confused. I don't know if this was
  an actual problem, but the code was like that before and it seems saner
  to me. Seeing there is no locking in the device add-on itself, we may
  already enter the code from the node monitor thread.

This should fix #2894.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@28321 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/input_server/devices/keyboard/KeyboardInputDevice.cpp

@@ -379,8 +379,8 @@ KeyboardInputDevice::KeyboardInputDevice()
 {
 	CALLED();
 
-	StartMonitoringDevice(kKeyboardDevicesDirectory);
 	_RecursiveScan(kKeyboardDevicesDirectory);
+	StartMonitoringDevice(kKeyboardDevicesDirectory);
 }
 
 
@@ -525,14 +525,32 @@ KeyboardInputDevice::_HandleMonitor(BMessage* message)
 	const char* path;
 	int32 opcode;
 	if (message->FindInt32("opcode", &opcode) != B_OK
-		|| opcode != B_ENTRY_CREATED && opcode != B_ENTRY_REMOVED
+		|| (opcode != B_ENTRY_CREATED && opcode != B_ENTRY_REMOVED)
 		|| message->FindString("path", &path) != B_OK)
 		return B_BAD_VALUE;
 
 	if (opcode == B_ENTRY_CREATED)
 		return _AddDevice(path);
 
+#if 0
 	return _RemoveDevice(path);
+#else
+	// Don't handle B_ENTRY_REMOVED, let the control thread take care of it.
+	return B_OK;
+#endif
+}
+
+
+keyboard_device*
+KeyboardInputDevice::_FindDevice(const char* path) const
+{
+	for (int i = fDevices.CountItems() - 1; i >= 0; i--) {
+		keyboard_device* device = fDevices.ItemAt(i);
+		if (strcmp(device->path, path) == 0)
+			return device;
+	}
+
+	return NULL;
 }
 
 
@@ -540,6 +558,9 @@ status_t
 KeyboardInputDevice::_AddDevice(const char* path)
 {
 	CALLED();
+
+	_RemoveDevice(path);
+
 	keyboard_device* device = new(std::nothrow) keyboard_device(path);
 	if (device == NULL)
 		return B_NO_MEMORY;
@@ -560,22 +581,20 @@ status_t
 KeyboardInputDevice::_RemoveDevice(const char* path)
 {
 	CALLED();
-	keyboard_device* device;
-	for (int i = 0; (device = fDevices.ItemAt(i)) != NULL; i++) {
-		if (!strcmp(device->path, path)) {
-			fDevices.RemoveItemAt(i);
 
-			input_device_ref* devices[2];
+	keyboard_device* device = _FindDevice(path);
-			devices[0] = &device->device_ref;
+	if (device == NULL)
-			devices[1] = NULL;
+		return B_ENTRY_NOT_FOUND;
-			UnregisterDevices(devices);
 
-			delete device;
+	input_device_ref* devices[2];
-			return B_OK;
+	devices[0] = &device->device_ref;
-		}
+	devices[1] = NULL;
-	}
 
-	return B_ENTRY_NOT_FOUND;
+	UnregisterDevices(devices);
+
+	fDevices.RemoveItem(device);
+
+	return B_OK;
 }
 
 
@@ -598,8 +617,12 @@ KeyboardInputDevice::_DeviceWatcher(void* arg)
 	LOG("%s\n", __PRETTY_FUNCTION__);
 
 	while (device->active) {
-		if (ioctl(device->fd, KB_READ, &buffer) != B_OK)
+		if (ioctl(device->fd, KB_READ, &buffer) != B_OK) {
+			device->device_watcher = -1;
+			device->owner->_RemoveDevice(device->path);
+			// TOAST!
 			return 0;
+		}
 
 		uint32 keycode = 0;
 		bool isKeyDown = false;
@@ -697,7 +720,7 @@ KeyboardInputDevice::_DeviceWatcher(void* arg)
 			msg->AddInt32("modifiers", device->modifiers);
 			msg->AddData("states", B_UINT8_TYPE, states, 16);
 
-			if (owner->EnqueueMessage(msg)!=B_OK)
+			if (owner->EnqueueMessage(msg) != B_OK)
 				delete msg;
 
 			if (modifiers & (B_CAPS_LOCK | B_NUM_LOCK | B_SCROLL_LOCK))
@@ -717,8 +740,8 @@ KeyboardInputDevice::_DeviceWatcher(void* arg)
 
 			BMessage* msg = new BMessage;
 			if (msg == NULL) {
-				free(string);
+				delete[] string;
-				free(rawString);
+				delete[] rawString;
 				continue;
 			}
 

src/add-ons/input_server/devices/keyboard/KeyboardInputDevice.h

@@ -60,10 +60,15 @@ public:
 	virtual status_t		SystemShuttingDown();
 
 private:
+	friend struct keyboard_device;
+	// TODO: needed by the control thread to remove a dead device
+	// find a better way...
+
 			status_t		_HandleMonitor(BMessage* message);
 			status_t		_InitFromSettings(void* cookie, uint32 opcode = 0);
 			void			_RecursiveScan(const char* directory);
 
+			keyboard_device* _FindDevice(const char* path) const;
 			status_t		_AddDevice(const char* path);
 			status_t		_RemoveDevice(const char* path);
 

src/add-ons/input_server/devices/mouse/MouseInputDevice.cpp

@@ -38,16 +38,20 @@
 #	define LOG_ERR(text...) debug_printf(text)
 #endif
 
+//#define LOG_DEVICES(text...) debug_printf(text)
+#define LOG_DEVICES(text...) LOG(text)
+
 #define CALLED() LOG("%s\n", __PRETTY_FUNCTION__)
 
 
+
 const static uint32 kMouseThreadPriority = B_FIRST_REAL_TIME_PRIORITY + 4;
 const static char* kMouseDevicesDirectory = "/dev/input/mouse";
 
 
 class MouseDevice {
 	public:
-		MouseDevice(BInputServerDevice& target, const char* path);
+		MouseDevice(MouseInputDevice& target, const char* path);
 		~MouseDevice();
 
 		status_t Start();
@@ -71,16 +75,16 @@ class MouseDevice {
 		char* _BuildShortName() const;
 
 	private:
-		BInputServerDevice& fTarget;
+		MouseInputDevice&	fTarget;
-		BString	fPath;
+		BString				fPath;
-		int fDevice;
+		int					fDevice;
 
-		input_device_ref fDeviceRef;
+		input_device_ref	fDeviceRef;
-		mouse_settings fSettings;
+		mouse_settings		fSettings;
-		bool fDeviceRemapsButtons;
+		bool				fDeviceRemapsButtons;
 
-		thread_id fThread;
+		thread_id			fThread;
-		volatile bool fActive;
+		volatile bool		fActive;
 };
 
 
@@ -94,15 +98,14 @@ instantiate_input_device()
 //	#pragma mark -
 
 
-MouseDevice::MouseDevice(BInputServerDevice& target, const char* driverPath)
+MouseDevice::MouseDevice(MouseInputDevice& target, const char* driverPath)
 	:
 	fTarget(target),
+	fPath(driverPath),
 	fDevice(-1),
 	fThread(-1),
 	fActive(false)
 {
-	fPath = driverPath;
-
 	fDeviceRef.name = _BuildShortName();
 	fDeviceRef.type = B_POINTING_DEVICE;
 	fDeviceRef.cookie = this;
@@ -230,8 +233,12 @@ MouseDevice::_Run()
 		mouse_movement movements;
 		memset(&movements, 0, sizeof(movements));
 
-		if (ioctl(fDevice, MS_READ, &movements) != B_OK)
+		if (ioctl(fDevice, MS_READ, &movements) != B_OK) {
+			fThread = -1;
+			fTarget._RemoveDevice(fPath.String());
+			// TOAST!
 			return;
+		}
 
 		uint32 buttons = lastButtons ^ movements.buttons;
 
@@ -244,8 +251,6 @@ MouseDevice::_Run()
 			movements.clicks, movements.wheel_xdelta, movements.wheel_ydelta);
 		LOG("%s: x: %ld, y: %ld\n", fDeviceRef.name, deltaX, deltaY);
 
-		BMessage* message = NULL;
-
 		// Send single messages for each event
 
 		if (buttons != 0) {
@@ -273,7 +278,7 @@ MouseDevice::_Run()
 		}
 
 		if ((movements.wheel_ydelta != 0) || (movements.wheel_xdelta != 0)) {
-			message = new BMessage(B_MOUSE_WHEEL_CHANGED);
+			BMessage* message = new BMessage(B_MOUSE_WHEEL_CHANGED);
 			if (message == NULL)
 				continue;
 
@@ -405,8 +410,8 @@ MouseInputDevice::MouseInputDevice()
 {
 	CALLED();
 
-	StartMonitoringDevice(kMouseDevicesDirectory);
 	_RecursiveScan(kMouseDevicesDirectory);
+	StartMonitoringDevice(kMouseDevicesDirectory);
 }
 
 
@@ -464,7 +469,6 @@ MouseInputDevice::Control(const char* name, void* cookie,
 }
 
 
-// TODO: Test this. USB doesn't work on my machine
 status_t
 MouseInputDevice::_HandleMonitor(BMessage* message)
 {
@@ -473,14 +477,19 @@ MouseInputDevice::_HandleMonitor(BMessage* message)
 	const char* path;
 	int32 opcode;
 	if (message->FindInt32("opcode", &opcode) != B_OK
-		|| opcode != B_ENTRY_CREATED && opcode != B_ENTRY_REMOVED
+		|| (opcode != B_ENTRY_CREATED && opcode != B_ENTRY_REMOVED)
 		|| message->FindString("path", &path) != B_OK)
 		return B_BAD_VALUE;
 
 	if (opcode == B_ENTRY_CREATED)
 		return _AddDevice(path);
 
+#if 0
 	return _RemoveDevice(path);
+#else
+	// Don't handle B_ENTRY_REMOVED, let the control thread take care of it.
+	return B_OK;
+#endif
 }
 
 
@@ -489,9 +498,9 @@ MouseInputDevice::_FindDevice(const char* path)
 {
 	CALLED();
 
-	for (int32 i = fDevices.CountItems(); i-- > 0;) {
+	for (int32 i = fDevices.CountItems() - 1; i >= 0; i--) {
 		MouseDevice* device = fDevices.ItemAt(i);
-		if (!strcmp(device->Path(), path))
+		if (strcmp(device->Path(), path) == 0)
 			return device;
 	}
 
@@ -504,9 +513,11 @@ MouseInputDevice::_AddDevice(const char* path)
 {
 	CALLED();
 
+	_RemoveDevice(path);
+
 	MouseDevice* device = new(std::nothrow) MouseDevice(*this, path);
 	if (!device) {
-		LOG("No memory\n");
+		LOG("MouseInputDevice::_AddDevice() - No memory\n");
 		return B_NO_MEMORY;
 	}
 
@@ -519,6 +530,9 @@ MouseInputDevice::_AddDevice(const char* path)
 	devices[0] = device->DeviceRef();
 	devices[1] = NULL;
 
+	LOG_DEVICES("MouseInputDevice::_AddDevice(%s), name: %s\n", path,
+		devices[0]->name);
+
 	return RegisterDevices(devices);
 }
 
@@ -532,15 +546,17 @@ MouseInputDevice::_RemoveDevice(const char* path)
 	if (device == NULL)
 		return B_ENTRY_NOT_FOUND;
 
-	fDevices.RemoveItem(device);
-
 	input_device_ref* devices[2];
 	devices[0] = device->DeviceRef();
 	devices[1] = NULL;
 
+	LOG_DEVICES("MouseInputDevice::_RemoveDevice(%s), name: %s\n", path,
+		devices[0]->name);
+
 	UnregisterDevices(devices);
 
-	delete device;
+	fDevices.RemoveItem(device);
+
 	return B_OK;
 }
 

src/add-ons/input_server/devices/mouse/MouseInputDevice.h

@@ -31,6 +31,10 @@ public:
 								uint32 command, BMessage* message);
 
 private:
+	friend class MouseDevice;
+	// TODO: needed by the control thread to remove a dead device
+	// find a better way...
+
 			status_t		_HandleMonitor(BMessage* message);
 			void			_RecursiveScan(const char* directory);