Correct the way serial devices are deleted. They are either deleted on removal

when they are not open or they are deleted on free when they are already
removed. This should fix the sudden crashes when you unplugged a device that
might not have been fully closed yet.
Also handle the case of removal correctly and don't use the usb_device anymore
after releasing it by returning from the device removed hook. Calls to the
device just return B_DEV_NOT_READY in that case.

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

src/add-ons/kernel/drivers/ports/usb_serial/Driver.cpp

@@ -87,8 +87,13 @@ usb_serial_device_removed(void *cookie)
 	SerialDevice *device = (SerialDevice *)cookie;
 	for (int32 i = 0; i < DEVICES_COUNT; i++) {
 		if (gSerialDevices[i] == device) {
-			delete device;
-			gSerialDevices[i] = NULL;
+			if (device->IsOpen()) {
+				// the device will be deleted upon being freed
+				device->Removed();
+			} else {
+				delete device;
+				gSerialDevices[i] = NULL;
+			}
 			break;
 		}
 	}
@@ -131,7 +136,7 @@ init_driver()
 	}
 
 	for (int32 i = 0; i < DEVICES_COUNT; i++)
-		gSerialDevices[i] = 0;
+		gSerialDevices[i] = NULL;
 
 	gDeviceNames[0] = NULL;
 
@@ -293,7 +298,22 @@ usb_serial_free(void *cookie)
 {
 	TRACE_FUNCALLS("> usb_serial_free(0x%08x)\n", cookie);
 	SerialDevice *device = (SerialDevice *)cookie;
-	return device->Free();
+	acquire_sem(gDriverLock);
+	status_t status = device->Free();
+	if (device->IsRemoved()) {
+		for (int32 i = 0; i < DEVICES_COUNT; i++) {
+			if (gSerialDevices[i] == device) {
+				// the device is removed already but as it was open the
+				// removed hook has not deleted the object
+				delete device;
+				gSerialDevices[i] = NULL;
+				break;
+			}
+		}
+	}
+
+	release_sem(gDriverLock);
+	return status;
 }
 
 

src/add-ons/kernel/drivers/ports/usb_serial/SerialDevice.cpp

@@ -19,6 +19,8 @@ SerialDevice::SerialDevice(usb_device device, uint16 vendorID,
 		fVendorID(vendorID),
 		fProductID(productID),
 		fDescription(description),
+		fDeviceOpen(false),
+		fDeviceRemoved(false),
 		fControlPipe(0),
 		fReadPipe(0),
 		fWritePipe(0),
@@ -43,19 +45,13 @@ SerialDevice::SerialDevice(usb_device device, uint16 vendorID,
 
 SerialDevice::~SerialDevice()
 {
-	fStopDeviceThread = true;
-	gUSBModule->cancel_queued_transfers(fReadPipe);
-	gUSBModule->cancel_queued_transfers(fWritePipe);
-	gUSBModule->cancel_queued_transfers(fControlPipe);
+	Removed();
 
 	if (fDoneRead >= B_OK)
 		delete_sem(fDoneRead);
 	if (fDoneWrite >= B_OK)
 		delete_sem(fDoneWrite);
 
-	int32 result = B_OK;
-	wait_for_thread(fDeviceThread, &result);
-
 	if (fBufferArea >= B_OK)
 		delete_area(fBufferArea);
 
@@ -250,6 +246,12 @@ SerialDevice::Service(struct tty *ptty, struct ddrover *ddr, uint flags)
 status_t
 SerialDevice::Open(uint32 flags)
 {
+	if (fDeviceOpen)
+		return B_BUSY;
+
+	if (fDeviceRemoved)
+		return B_DEV_NOT_READY;
+
 	gTTYModule->ttyinit(&fTTY, true);
 	fTTYFile.tty = &fTTY;
 	fTTYFile.flags = flags;
@@ -285,17 +287,18 @@ SerialDevice::Open(uint32 flags)
 		fInterruptBufferSize, InterruptCallbackFunction, this);
 	if (status < B_OK)
 		TRACE_ALWAYS("failed to queue initial interrupt\n");
-	return status;
+
+	fDeviceOpen = true;
+	return B_OK;
 }
 
 
 status_t
 SerialDevice::Read(char *buffer, size_t *numBytes)
 {
-	struct ddrover *ddr = gTTYModule->ddrstart(NULL);
-	if (!ddr) {
+	if (fDeviceRemoved) {
 		*numBytes = 0;
-		return B_NO_MEMORY;
+		return B_DEV_NOT_READY;
 	}
 
 	status_t status = benaphore_lock(&fReadLock);
@@ -305,6 +308,13 @@ SerialDevice::Read(char *buffer, size_t *numBytes)
 		return status;
 	}
 
+	struct ddrover *ddr = gTTYModule->ddrstart(NULL);
+	if (!ddr) {
+		*numBytes = 0;
+		benaphore_unlock(&fReadLock);
+		return B_NO_MEMORY;
+	}
+
 	status = gTTYModule->ttyread(&fTTYFile, ddr, buffer, numBytes);
 	gTTYModule->ddrdone(ddr);
 
@@ -325,6 +335,11 @@ SerialDevice::Write(const char *buffer, size_t *numBytes)
 		return status;
 	}
 
+	if (fDeviceRemoved) {
+		benaphore_unlock(&fWriteLock);
+		return B_DEV_NOT_READY;
+	}
+
 	while (bytesLeft > 0) {
 		size_t length = MIN(bytesLeft, fWriteBufferSize);
 		OnWrite(buffer, &length);
@@ -367,6 +382,9 @@ SerialDevice::Write(const char *buffer, size_t *numBytes)
 status_t
 SerialDevice::Control(uint32 op, void *arg, size_t length)
 {
+	if (fDeviceRemoved)
+		return B_DEV_NOT_READY;
+
 	struct ddrover *ddr = gTTYModule->ddrstart(NULL);
 	if (!ddr)
 		return B_NO_MEMORY;
@@ -380,6 +398,9 @@ SerialDevice::Control(uint32 op, void *arg, size_t length)
 status_t
 SerialDevice::Select(uint8 event, uint32 ref, selectsync *sync)
 {
+	if (fDeviceRemoved)
+		return B_DEV_NOT_READY;
+
 	struct ddrover *ddr = gTTYModule->ddrstart(NULL);
 	if (!ddr)
 		return B_NO_MEMORY;
@@ -393,6 +414,9 @@ SerialDevice::Select(uint8 event, uint32 ref, selectsync *sync)
 status_t
 SerialDevice::DeSelect(uint8 event, selectsync *sync)
 {
+	if (fDeviceRemoved)
+		return B_DEV_NOT_READY;
+
 	struct ddrover *ddr = gTTYModule->ddrstart(NULL);
 	if (!ddr)
 		return B_NO_MEMORY;
@@ -408,9 +432,11 @@ SerialDevice::Close()
 {
 	OnClose();
 
-	gUSBModule->cancel_queued_transfers(fReadPipe);
-	gUSBModule->cancel_queued_transfers(fWritePipe);
-	gUSBModule->cancel_queued_transfers(fControlPipe);
+	if (!fDeviceRemoved) {
+		gUSBModule->cancel_queued_transfers(fReadPipe);
+		gUSBModule->cancel_queued_transfers(fWritePipe);
+		gUSBModule->cancel_queued_transfers(fControlPipe);
+	}
 
 	struct ddrover *ddr = gTTYModule->ddrstart(NULL);
 	if (!ddr)
@@ -418,6 +444,8 @@ SerialDevice::Close()
 
 	status_t status = gTTYModule->ttyclose(&fTTYFile, ddr);
 	gTTYModule->ddrdone(ddr);
+
+	fDeviceOpen = false;
 	return status;
 }
 
@@ -435,6 +463,31 @@ SerialDevice::Free()
 }
 
 
+void
+SerialDevice::Removed()
+{
+	if (fDeviceRemoved)
+		return;
+
+	// notifies us that the device was removed
+	fDeviceRemoved = true;
+
+	// we need to ensure that we do not use the device anymore
+	fStopDeviceThread = true;
+	fInputStopped = false;
+	gUSBModule->cancel_queued_transfers(fReadPipe);
+	gUSBModule->cancel_queued_transfers(fWritePipe);
+	gUSBModule->cancel_queued_transfers(fControlPipe);
+
+	int32 result = B_OK;
+	wait_for_thread(fDeviceThread, &result);
+	fDeviceThread = -1;
+
+	benaphore_lock(&fWriteLock);
+	benaphore_unlock(&fWriteLock);
+}
+
+
 status_t
 SerialDevice::AddDevice(const usb_configuration_info *config)
 {
@@ -586,7 +639,7 @@ SerialDevice::InterruptCallbackFunction(void *cookie, int32 status,
 
 	// ToDo: maybe handle those somehow?
 
-	if (status == B_OK) {
+	if (status == B_OK && !device->fDeviceRemoved) {
 		status = gUSBModule->queue_interrupt(device->fControlPipe,
 			device->fInterruptBuffer, device->fInterruptBufferSize,
 			device->InterruptCallbackFunction, device);

src/add-ons/kernel/drivers/ports/usb_serial/SerialDevice.h

@@ -55,6 +55,10 @@ static	SerialDevice *			MakeDevice(usb_device device, uint16 vendorID,
 		status_t				Close();
 		status_t				Free();
 
+		bool					IsOpen() { return fDeviceOpen; };
+		void					Removed();
+		bool					IsRemoved() { return fDeviceRemoved; };
+
 		/* virtual interface to be overriden as necessary */
 virtual	status_t				AddDevice(const usb_configuration_info *config);
 
@@ -84,6 +88,8 @@ static	void					InterruptCallbackFunction(void *cookie,
 		uint16					fVendorID;
 		uint16					fProductID;
 		const char *			fDescription;	// informational description
+		bool					fDeviceOpen;
+		bool					fDeviceRemoved;
 
 		/* communication pipes */
 		usb_pipe				fControlPipe;