NanoPangBZ / win_serial_port_cpp_class

使用win api实现的windows串口类
0 stars 0 forks source link

Bug #4

Closed NanoPangBZ closed 11 months ago

NanoPangBZ commented 11 months ago

_connect函数返回没有解锁,导致connect失败后再次connect会在导致线程死锁

codeautopilot[bot] commented 11 months ago

Potential solution

The plan is to modify the _connect function to ensure that mutex locks are properly released in case of a failure during the connection process. This will prevent a deadlock when a new connection attempt is made after a failure. The reasoning behind this solution is that if a lock is not released after a failure, the next time _connect is called, it will try to acquire the same lock again, which could lead to a deadlock since the lock is already held by the thread that encountered the failure.

Code

Here is the implementation of the changes to the _connect function in the SerialPort.cpp file:

bool SerialPort::_connect()
{
    _hSerialReadLock->lock();
    _hSerialWriteLock->lock();

    std::string _fileName = "\\\\.\\";  // To access the device name
    _fileName.append(_serialInfo.portName.data());

    _hSerial = CreateFile((LPCSTR)_fileName.c_str(), GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
    if (_hSerial == INVALID_HANDLE_VALUE) {
        std::cerr << "Failed to open serial port." << std::endl;
        _hSerialReadLock->unlock();
        _hSerialWriteLock->unlock();
        return false;
    }

    _dcbSerialParams.DCBlength = sizeof(_dcbSerialParams);
    if (!GetCommState(_hSerial, &_dcbSerialParams)) {
        std::cerr << "Error in GetCommState." << std::endl;
        CloseHandle(_hSerial);
        _hSerialReadLock->unlock();
        _hSerialWriteLock->unlock();
        return false;
    }

    // ... (rest of the configuration code)

    if (!SetCommTimeouts(_hSerial, &timeouts)) {
        std::cerr << "Failed to set serial port timeouts." << std::endl;
        CloseHandle(_hSerial);
        _hSerialReadLock->unlock();
        _hSerialWriteLock->unlock();
        return false;
    }

    // Establish asynchronous receive/callback function thread
    _recieveThread = new std::thread(&SerialPort::_recieveThreadImpl, this);
    _recieveThread->detach();   // Detach the thread

    _hSerialReadLock->unlock();
    _hSerialWriteLock->unlock();

    return true;
}

This code ensures that the locks are released in every failure scenario within the _connect function. It is important to note that the locks should also be released at the end of the function if the connection is successful, as shown in the last part of the code snippet.

Changes on SerialPort/SerialPort.cpp To address the issue reported by the user, we need to ensure that the mutex locks acquired in the `_connect` function are properly released in the event of a failure to connect. This will prevent a deadlock on subsequent connection attempts. Here's the plan to modify the `_connect` function in the `SerialPort.cpp` file: 1. Acquire the locks at the beginning of the `_connect` function using `_hSerialReadLock->lock()` and `_hSerialWriteLock->lock()`. 2. After each critical section where a failure can occur (e.g., `CreateFile`, `GetCommState`, `SetCommState`, `SetCommTimeouts`), check if the operation failed. 3. If an operation fails, before returning `false`, release the locks by calling `_hSerialReadLock->unlock()` and `_hSerialWriteLock->unlock()`. 4. Ensure that the locks are also released at the end of the `_connect` function if the connection is successful. Here is the modified `_connect` function with the necessary changes: ```cpp bool SerialPort::_connect() { _hSerialReadLock->lock(); _hSerialWriteLock->lock(); std::string _fileName = "\\\\.\\"; // To access the device name _fileName.append(_serialInfo.portName.data()); _hSerial = CreateFile((LPCSTR)_fileName.c_str(), GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); if (_hSerial == INVALID_HANDLE_VALUE) { std::cerr << "Failed to open serial port." << std::endl; _hSerialReadLock->unlock(); _hSerialWriteLock->unlock(); return false; } _dcbSerialParams.DCBlength = sizeof(_dcbSerialParams); if (!GetCommState(_hSerial, &_dcbSerialParams)) { std::cerr << "Error in GetCommState." << std::endl; CloseHandle(_hSerial); _hSerialReadLock->unlock(); _hSerialWriteLock->unlock(); return false; } // ... (rest of the configuration code) if (!SetCommTimeouts(_hSerial, &timeouts)) { std::cerr << "Failed to set serial port timeouts." << std::endl; CloseHandle(_hSerial); _hSerialReadLock->unlock(); _hSerialWriteLock->unlock(); return false; } // Establish asynchronous receive/callback function thread _recieveThread = new std::thread(&SerialPort::_recieveThreadImpl, this); _recieveThread->detach(); // Detach the thread _hSerialReadLock->unlock(); _hSerialWriteLock->unlock(); return true; } ``` With these changes, the locks will be properly released in case of a failure, allowing subsequent connection attempts to proceed without causing a deadlock.
NanoPangBZ commented 11 months ago

没有这个bug,误检