Чистое закрытие QSerialPort в Qt

Question

Я пытаюсь закрыть последовательный порт, открытый с помощью библиотеки QSerialPort, но он висит более чем в половину времени.

Я разрабатываю многопоточное приложение с одним потоком, отвечающим за интерфейс, а другой для последовательной связи. Я использую класс оболочки QThread.

void CommThread::run() 
{ 
    serial = new QSerialPort(); 

    serial->setPortName(portname); 
    serial->setBaudRate(QSerialPort::Baud115200); 

    if(!serial->open(QIODevice::ReadWrite)){ 
     qDebug() << "Error opening Serial port within thread"; 
     quit = true; 
     return; 
    }else{ 
     /// \todo handle this exception more gracefully 
    } 

    /// Start our reading loop 
    /// While CommThread::disconnect is not called, this loop will run 
    while(!quit){ 
     comm_mutex->lock(); 

     /// If CommThread::disconnect() is called send DISCONNECT Packet 
     if(dconnect){ 
      // Signal device to disconnect so that it can suspend USB CDC transmission of data 
      qDebug() << "Entering disconnect sequence"; 

      serial->write(data); 
      serial->flush(); 

      break; 
     } 

     /// No write or disconnect requested 
     /// Read incoming data from port 
     if(serial->waitForReadyRead(-1)){ 
      if(serial->canReadLine()){ 
       // Read stuff here 
      } 
     } 

     // Transform the stuff read here 

     comm_mutex->lock() 
     // Do something to a shared data structure 
     // emit signal to main thread that data is ready   
     comm_mutex->unlock(); 
    } 

    comm_mutex->unlock(); 

    // Thread is exiting, clean up resources it created 
    qDebug() << "Thread ID" << QThread::currentThreadId(); 
    qDebug() << "Thread:: Closing and then deleting the serial port"; 
    qDebug() << "Lets check the error string" << serial->errorString(); 
    delete comm_mutex; 
    serial->close(); 
    qDebug() << "Thread:: Port closed"; 
    delete serial; 
    qDebug() << "Thread:: Serial deleted"; 
    delete img; 
    qDebug() << "Thread:: Image deleted"; 
    qDebug() << "Thread:: Serial port and img memory deleted"; 
    quit = true; 

} 

Проблема заключается в том, когда поток UI устанавливает dconnect переменного истинные и продолжает удалять нить связи он застревает в деструкторе нити связи, которая выглядит следующим образом:

CommThread::~CommThread() 
{ 
    qDebug() << "Destructor waiting for thread to stop"; 
    QThread::wait(); 
    qDebug() << "Destuctor Commthread ID" << QThread::currentThreadId(); 
    qDebug() << "Commthread wrapper exiting"; 
} 

2 из трех раз линия связи висит на линии serial-close(), в результате чего поток пользовательского интерфейса висит на линии QThread::wait() в деструкторе. Излишне говорить, что это приводит к замороженному пользовательскому интерфейсу и, если оно закрыто, все приложение остается в памяти до тех пор, пока не будет убито диспетчером задач. Через несколько минут вызов serial :: close(), наконец, вернется; что я хотел бы знать, это то, что не так, и как я могу лучше избегать висячего интерфейса?

Я изучил код QSerialPort, и я не вижу ничего явно неправильного. Если я звоню serial->errorCode(), я получаю строку UknownError, но это происходит даже тогда, когда порт закрывается без зависаний.

EDIT: Это НИКОГДА не происходит в отладчике. SerialPort всегда закрывается сразу и деструкторов парусами через без каких-либо зависаний на QThread :: ждать()

EDIT: Я уверен, что это Serial-> близко(), который висит, потому что я могу видеть QDebug (), который печатается непосредственно перед его зависанием в течение нескольких секунд или минут).

Устройство прекращает передачу, поскольку в коммутаторе dconnect отключается пакет разъединения, а светодиод на устройстве становится зеленым.

Answer 1

Несколько вещей:

1. Можно, конечно, просто утечка порт, если он не закрывается достаточно скоро.

2. Вы должны выполнить изящный выход, когда пользовательский интерфейс является отзывчивым, и при отключении потока выполняется таймаут.

3. Для управления ресурсами вы должны использовать интеллектуальные указатели и другие методы RAII. Это C++, а не C. Идеально хранить вещи по значению, а не через указатель.

4. Вы не должны блокировать разделы, в которых вы изменяете общую структуру данных под блокировкой.

5. Вы должны уведомлять об изменениях в структуре данных (возможно, вы это делаете). Как другой код может зависеть от таких изменений без опроса в противном случае? Он не может, и опрос ужасен для производительности.

6. QThread предлагает requestInterruption и isInterruptionRequested код, который переопределяет run без цикла событий. Используйте его, не откатывайте свои выигранные флаги quit.

7. Ваш код будет намного проще, если вы использовали QObject напрямую.

Как минимум, нам нужен пользовательский интерфейс, который не будет блокироваться при отключении рабочего потока. Мы начинаем с реализации потока, которая имеет функциональные возможности, необходимые для поддержки такого пользовательского интерфейса.

// https://github.com/KubaO/stackoverflown/tree/master/questions/serial-test-32331713 
#include <QtWidgets> 

/// A thread that gives itself a bit of time to finish up, and then terminates. 
class Thread : public QThread { 
    Q_OBJECT 
    Q_PROPERTY (int shutdownTimeout MEMBER m_shutdownTimeout) 
    int m_shutdownTimeout { 1000 }; ///< in milliseconds 
    QBasicTimer m_shutdownTimer; 
    void timerEvent(QTimerEvent * ev) override { 
     if (ev->timerId() == m_shutdownTimer.timerId()) { 
     if (! isFinished()) terminate(); 
     } 
     QThread::timerEvent(ev); 
    } 
    bool event(QEvent *event) override { 
     if (event->type() == QEvent::ThreadChange) 
     QCoreApplication::postEvent(this, new QEvent(QEvent::None)); 
     else if (event->type() == QEvent::None && thread() == currentThread()) 
     // Hint that moveToThread(this) is an antipattern 
     qWarning() << "The thread controller" << this << "is running in its own thread."; 
     return QThread::event(event); 
    } 
    using QThread::requestInterruption; ///< Hidden, use stop() instead. 
    using QThread::quit; ///< Hidden, use stop() instead. 
public: 
    Thread(QObject * parent = 0) : QThread(parent) { 
     connect(this, &QThread::finished, this, [this]{ m_shutdownTimer.stop(); }); 
    } 
    /// Indicates that the thread is attempting to finish. 
    Q_SIGNAL void stopping(); 
    /// Signals the thread to stop in a general way. 
    Q_SLOT void stop() { 
     emit stopping(); 
     m_shutdownTimer.start(m_shutdownTimeout, this); 
     requestInterruption(); // should break a run() that has no event loop 
     quit();    // should break the event loop if there is one 
    } 
    ~Thread() { 
     Q_ASSERT(!thread() || thread() == QThread::currentThread()); 
     stop(); 
     wait(50); 
     if (isRunning()) terminate(); 
     wait(); 
    } 
}; 

Это немного лжи, что Thread есть-QThread, так как мы не можем использовать некоторые из членов базового класса на нем, тем самым нарушая LSP. В идеале Thread должен быть QObject и содержать только QThread.

Затем мы реализуем фиктивную нить, которая занимает свое время, чтобы завершить работу, и может необязательно застревать навсегда, точно так же, как когда-то ваш код (хотя это не обязательно).

class LazyThread : public Thread { 
    Q_OBJECT 
    Q_PROPERTY(bool getStuck MEMBER m_getStuck) 
    bool m_getStuck { false }; 
    void run() override { 
     while (!isInterruptionRequested()) { 
     msleep(100); // pretend that we're busy 
     } 
     qDebug() << "loop exited"; 
     if (m_getStuck) { 
     qDebug() << "stuck"; 
     Q_FOREVER sleep(1); 
     } else { 
     qDebug() << "a little nap"; 
     sleep(2); 
     } 
    } 
public: 
    LazyThread(QObject * parent = 0) : Thread(parent) { 
     setProperty("shutdownTimeout", 5000); 
    } 
}; 

Затем нам нужен класс, который может связывать рабочие потоки и запросы пользовательского интерфейса. Он устанавливает себя как фильтр событий в главном окне и задерживает его закрытие до тех пор, пока все потоки не прекратятся.

class CloseThreadStopper : public QObject { 
    Q_OBJECT 
    QSet<Thread*> m_threads; 
    void done(Thread* thread){ 
     m_threads.remove(thread); 
     if (m_threads.isEmpty()) emit canClose(); 
    } 
    bool eventFilter(QObject * obj, QEvent * ev) override { 
     if (ev->type() == QEvent::Close) { 
     bool close = true; 
     for (auto thread : m_threads) { 
      if (thread->isRunning() && !thread->isFinished()) { 
       close = false; 
       ev->ignore(); 
       connect(thread, &QThread::finished, this, [this, thread]{ done(thread); }); 
       thread->stop(); 
      } 
     } 
     return !close; 
     } 
     return false; 
    } 
public: 
    Q_SIGNAL void canClose(); 
    CloseThreadStopper(QObject * parent = 0) : QObject(parent) {} 
    void addThread(Thread* thread) { 
     m_threads.insert(thread); 
     connect(thread, &QObject::destroyed, this, [this, thread]{ done(thread); }); 
    } 
    void installOn(QWidget * w) { 
     w->installEventFilter(this); 
     connect(this, &CloseThreadStopper::canClose, w, &QWidget::close); 
    } 
}; 

Наконец, у нас есть простой пользовательский интерфейс, который позволяет нам контролировать все это и видеть, что он работает. Ни в коем случае пользовательский интерфейс не реагирует или блокируется.

int main(int argc, char *argv[]) 
{ 
    QApplication a { argc, argv }; 
    LazyThread thread; 
    CloseThreadStopper stopper; 
    stopper.addThread(&thread); 

    QWidget ui; 
    QGridLayout layout { &ui }; 
    QLabel state; 
    QPushButton start { "Start" }, stop { "Stop" }; 
    QCheckBox stayStuck { "Keep the thread stuck" }; 
    layout.addWidget(&state, 0, 0, 1, 2); 
    layout.addWidget(&stayStuck, 1, 0, 1, 2); 
    layout.addWidget(&start, 2, 0); 
    layout.addWidget(&stop, 2, 1); 
    stopper.installOn(&ui); 
    QObject::connect(&stayStuck, &QCheckBox::toggled, &thread, [&thread](bool v){ 
     thread.setProperty("getStuck", v); 
    }); 

    QStateMachine sm; 
    QState s_started { &sm }, s_stopping { &sm }, s_stopped { &sm }; 
    sm.setGlobalRestorePolicy(QState::RestoreProperties); 
    s_started.assignProperty(&state, "text", "Running"); 
    s_started.assignProperty(&start, "enabled", false); 
    s_stopping.assignProperty(&state, "text", "Stopping"); 
    s_stopping.assignProperty(&start, "enabled", false); 
    s_stopping.assignProperty(&stop, "enabled", false); 
    s_stopped.assignProperty(&state, "text", "Stopped"); 
    s_stopped.assignProperty(&stop, "enabled", false); 

    for (auto state : { &s_started, &s_stopping }) 
     state->addTransition(&thread, SIGNAL(finished()), &s_stopped); 
    s_started.addTransition(&thread, SIGNAL(stopping()), &s_stopping); 
    s_stopped.addTransition(&thread, SIGNAL(started()), &s_started); 
    QObject::connect(&start, &QPushButton::clicked, [&]{ thread.start(); }); 
    QObject::connect(&stop, &QPushButton::clicked, &thread, &Thread::stop); 
    sm.setInitialState(&s_stopped); 

    sm.start(); 
    ui.show(); 
    return a.exec(); 
} 

#include "main.moc" 

Учитывая Thread класс, и после консультации выше (кроме точки 7), ваш run() должен выглядеть примерно следующим образом:

class CommThread : public Thread { 
    Q_OBJECT 
public: 
    enum class Request { Disconnect }; 
private: 
    QMutex m_mutex; 
    QQueue<Request> m_requests; 
    //... 
    void run() override; 
}; 

void CommThread::run() 
{ 
    QString portname; 
    QSerialPort port; 

    port.setPortName(portname); 
    port.setBaudRate(QSerialPort::Baud115200); 

    if (!port.open(QIODevice::ReadWrite)){ 
     qWarning() << "Error opening Serial port within thread"; 
     return; 
    } 

    while (! isInterruptionRequested()) { 
     QMutexLocker lock(&m_mutex); 
     if (! m_requests.isEmpty()) { 
     auto request = m_requests.dequeue(); 
     lock.unlock(); 
     if (request == Request::Disconnect) { 
      qDebug() << "Entering disconnect sequence"; 
      QByteArray data; 
      port.write(data); 
      port.flush(); 
     } 
     //... 
     } 
     lock.unlock(); 

     // The loop must run every 100ms to check for new requests 
     if (port.waitForReadyRead(100)) { 
     if (port.canReadLine()) { 
      //... 
     } 
     QMutexLocker lock(&m_mutex); 
     // Do something to a shared data structure 
     } 

     qDebug() << "The thread is exiting"; 
    } 
} 

Конечно, это действительно ужасный стиль, который излишне вращает цикл, ожидая, когда что-то случится, и т. д. Вместо этого тривиальный способ подхода к таким проблемам состоит в том, чтобы иметь QObject с потокобезопасным интерфейсом, который можно перенести на рабочий t hread.

Во-первых, любопытно повторяющийся помощник; см. this question для деталей.

namespace { 
template <typename F> 
static void postTo(QObject * obj, F && fun) { 
    QObject signalSource; 
    QObject::connect(&signalSource, &QObject::destroyed, obj, std::forward<F>(fun), 
        Qt::QueuedConnection); 
} 
} 

Мы выводим из QObject и использовать postTo для выполнения функторов из цикла событий нашего потока.

class CommObject : public QObject { 
    Q_OBJECT 
    Q_PROPERTY(QImage image READ image NOTIFY imageChanged) 
    mutable QMutex m_imageMutex; 
    QImage m_image; 
    QByteArray m_data; 
    QString m_portName; 
    QSerialPort m_port { this }; 
    void onData() { 
     if (m_port.canReadLine()) { 
     // process the line 
     } 
     QMutexLocker lock(&m_imageMutex); 
     // Do something to the image 
     emit imageChanged(m_image); 
    } 
public: 
    /// Thread-safe 
    Q_SLOT void disconnect() { 
     postTo(this, [this]{ 
     qDebug() << "Entering disconnect sequence"; 
     m_port.write(m_data); 
     m_port.flush(); 
     }); 
    } 
    /// Thread-safe 
    Q_SLOT void open() { 
     postTo(this, [this]{ 
     m_port.setPortName(m_portName); 
     m_port.setBaudRate(QSerialPort::Baud115200); 
     if (!m_port.open(QIODevice::ReadWrite)){ 
      qWarning() << "Error opening the port"; 
      emit openFailed(); 
     } else { 
      emit opened(); 
     } 
     }); 
    } 
    Q_SIGNAL void opened(); 
    Q_SIGNAL void openFailed(); 
    Q_SIGNAL void imageChanged(const QImage &); 
    CommObject(QObject * parent = 0) : QObject(parent) { 
     open(); 
     connect(&m_port, &QIODevice::readyRead, this, &CommObject::onData); 
    } 
    QImage image() const { 
     QMutexLocker lock(&m_imageMutex); 
     return m_image; 
    } 
}; 

Заметим, что любой QIODevice автоматически закрывается при уничтожении. Таким образом, все, что нам нужно сделать, чтобы закрыть порт, - это уничтожить его в нужном рабочем потоке, чтобы длительная операция не блокировала пользовательский интерфейс.

Таким образом, мы действительно хотим, чтобы объект (и его порт) был удален в потоке (или утечке). Это просто достигается путем подключения Thread::stopping к слоту deleteLater объекта. Там закрытие порта может занимать столько же времени, сколько необходимо - Thread прекратит выполнение, если он истечет. Все время пользовательский интерфейс остается отзывчивым.

int main(...) { 
    //... 
    Thread thread; 
    thread.start(); 
    QScopedPointer<CommObject> comm(new CommObject); 
    comm->moveToThread(&thread); 
    QObject::connect(&thread, &Thread::stopping, comm.take(), &QObject::deleteLater); 
    //... 
}