From dbcfdd5ca72fcd9910b65a9061df902f78b2b3d6 Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Fri, 11 Apr 2008 21:52:39 +0000 Subject: [PATCH] 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 --- .../drivers/ports/usb_serial/Driver.cpp | 28 ++++++- .../drivers/ports/usb_serial/SerialDevice.cpp | 83 +++++++++++++++---- .../drivers/ports/usb_serial/SerialDevice.h | 6 ++ 3 files changed, 98 insertions(+), 19 deletions(-) diff --git a/src/add-ons/kernel/drivers/ports/usb_serial/Driver.cpp b/src/add-ons/kernel/drivers/ports/usb_serial/Driver.cpp index 0ce072a9ef..4cd2e2d380 100644 --- a/src/add-ons/kernel/drivers/ports/usb_serial/Driver.cpp +++ b/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; } diff --git a/src/add-ons/kernel/drivers/ports/usb_serial/SerialDevice.cpp b/src/add-ons/kernel/drivers/ports/usb_serial/SerialDevice.cpp index 419ed885eb..6eda26abcf 100644 --- a/src/add-ons/kernel/drivers/ports/usb_serial/SerialDevice.cpp +++ b/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); diff --git a/src/add-ons/kernel/drivers/ports/usb_serial/SerialDevice.h b/src/add-ons/kernel/drivers/ports/usb_serial/SerialDevice.h index ce8f923039..31914e7359 100644 --- a/src/add-ons/kernel/drivers/ports/usb_serial/SerialDevice.h +++ b/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;