3

Can someone tell me why this qt code will not call the Callback when ASYNC_TIMERS is defined (ie m_timer.start is called from the pthread but the slot never runs). Obviously it is to do with being called from a pthread as it works when ASYNC_TIMERS is not defined but I want to know how to fix it from the pthread. I have tryed many things found on the net including moveToThread(), calling the threads run which calls exec() but I have had no luck on this problem?

Cheers

multitimer.h:

#pragma once

#ifndef MULTI_TIMER_H
#define MULTI_TIMER_H

#include <QThread>
#include <QTimer>
#include <QMutex>

#include <QMap>


#include <QMetaType>
#include <cassert>


class MultiTimer : public QThread
{
    Q_OBJECT

public:
    typedef void (*multiTimerCallback)(quint32 p_id);

private:
    QTimer m_timer;
    QMutex m_mutex;
    quint32 m_id;
    multiTimerCallback m_callback;
    void KillTimer(void);

public:
    // only TimerFactory is allowed to instantiate MultiTimer
    MultiTimer(quint32 p_id, multiTimerCallback p_callback);
    ~MultiTimer();
    enum TTimerType
    {
        TT_SingleShot,      ///< Timer fires only once
        TT_Repetitive       ///< Timer keeps firing repeatedly until stopped with KillTimer()
    };
    void SetTimer(quint32 p_delayInMilliseconds, MultiTimer::TTimerType timerType);

private slots:
    void Update(void);
};

#endif

timer.cpp:

#include <QtCore/QCoreApplication>
#include "multitimer.h"
#include <stdio.h>


//--------------------------------------------------------------------------------------------------

void MultiTimer::SetTimer(quint32 p_delayInMilliseconds, MultiTimer::TTimerType timerType)
{
    QMutexLocker locker(&m_mutex);

    m_timer.setSingleShot(TT_SingleShot == timerType ? true : false);
    m_timer.start(p_delayInMilliseconds);
    //QTimer::singleShot(p_delayInMilliseconds, this,SLOT(Update()));
}

void MultiTimer::KillTimer(void)
{
    QMutexLocker locker(&m_mutex);
    m_timer.stop();
}

void MultiTimer::Update(void)
{
    QMutexLocker locker(&m_mutex);

    if (NULL != m_callback)
        m_callback(m_id);
}

MultiTimer::MultiTimer(quint32 p_id, multiTimerCallback p_callback)
    : m_id(p_id)
    , m_callback(p_callback)
{
    bool isConnected = true;
    isConnected &= this->connect(&this->m_timer, SIGNAL(timeout()), this, SLOT(Update()), Qt::QueuedConnection);
    assert(isConnected);
    //this->start();
}

MultiTimer::~MultiTimer()
{
    KillTimer();
    wait();
}


//--------------------------------------------------------------------------------------------------
#define ASYNC_TIMERS
#define GLOBAL_TIMERS

void Callback(quint32 p_id)
{
    printf("Got timered by timer %d.\n", p_id);
}

MultiTimer *mt;
void StartTimers(void)
{
    #ifndef GLOBAL_TIMERS
    mt = new MultiTimer(1, Callback);
    #endif
    mt->SetTimer(1000, MultiTimer::TT_SingleShot);
}

#ifdef ASYNC_TIMERS
pthread_t AsyncTaskThread;
void *ProcessAsyncTasks(void */*ptr*/)
{
    StartTimers();
    return NULL;
}
#endif

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);

    #ifdef GLOBAL_TIMERS
    mt = new MultiTimer(1, Callback);
    #endif

    #ifdef ASYNC_TIMERS
    pthread_create(&AsyncTaskThread, NULL, &ProcessAsyncTasks, NULL);
    #else
    StartTimers();
    #endif

    return a.exec();
}
othane
  • 143
  • 1
  • 5

3 Answers3

4

I think Threads and QObjects has the answer: you can't use event-driven objects in a different thread than where you created it.

In your code, if GLOBAL_TIMERS is enabled, you'll be creating the MultiTimer in your main thread, but calling m_timer.start() in a different one.

Quote the docs:

Event driven objects may only be used in a single thread. Specifically, this applies to the timer mechanism and the network module. For example, you cannot start a timer or connect a socket in a thread that is not the object's thread.

So don't do that. (And use QThread, while you're at it.)

Mat
  • 202,337
  • 40
  • 393
  • 406
  • I don't see him using events? – Frank Osterfeld May 18 '11 at 06:15
  • `m_timer.start(p_delayInMilliseconds);` will generate events after some it (if it worked in that context, which it doesn't) – Mat May 18 '11 at 06:19
  • When GLOBAL_TIMERS is defined, it does work. This is because the Multitimer object was created in a thread with an event loop. Note that he asks why it doesnt work when ASYNC_TIMERS is defined. That version doesn't work because there is no event loop in the thread in which he creates the MultiTimer object. – 0xbaadf00d May 18 '11 at 06:24
  • @justanothercoder: I don't agree with that completely. Your answer is correct, but from how I read the docs, with both async and global timers (i.e. with the code posted), he'd still be starting the timer from a different thread that created it. – Mat May 18 '11 at 06:26
  • If i read his code correctly, with GLOBAL_TIMERS enabled, he doesn't use any threading. And with ASYNC_TIMERS he creates and uses the MultiTimer object from the same, not main, thread. So, from what i see, this isn't the case here. EDIT - I'll read the code once more :) – 0xbaadf00d May 18 '11 at 06:28
  • @justanothercoder: with GLOBAL_TIMERS, `mt` is created in the main thread, then if ASYNC_TIMERS is defined `StartTimers` is called in the newly created plain pthread. – Mat May 18 '11 at 06:30
  • Doesn't StartTimers create a new MultiTimer object if ASYNC_TIMERS is defined? This would cause a memory leak if both of those are defined – 0xbaadf00d May 18 '11 at 06:34
  • @justanothercode: `#define ASYNC_TIMERS` `#define GLOBAL_TIMERS` is there in that code snippet... – Mat May 18 '11 at 06:34
  • @justanothercoder: the `new MultiTimer` in start timer is only **ifnedf** GLOBAL_TIMERS. – Mat May 18 '11 at 06:35
  • Ah, you are right (+1), i was still thinking that both wouldn't be defined at the same time. But i think that it wouldn't work even if only ASYNC_TIMERS was defined because there is no event loop in the thread in which he creates the QObject derived object. – 0xbaadf00d May 18 '11 at 06:39
  • @justanothercoder: yes, you are correct. an event loop is necessary, and he needs to be more careful with his object's initial threads. – Mat May 18 '11 at 06:40
  • #define GLOBAL_TIMERS does not seem to effect the results. It fails in the same way whether it is defined or not. Only #define ASYNC_TIMERS makes the difference, if defined the slot is never run but if not defined the slot is called as expected. – othane May 18 '11 at 23:03
  • @othane: read justanothercoder's reply. you _need_ an event loop, and you _need_ to be very careful with where you create your objects. you cannot just fire a pthread and expect things to work. – Mat May 18 '11 at 23:06
  • @Mat I did try something along those lines. In the MultiTimer I added a run method (since it is also a qthread) and started the thread in the constructor (and also tried in the SetTimer call) but that did not seem to work. I know this is not quit the same as having a runloop in the pthread but since in our real application the pthread is from a external library I cannot control this. So perhaps the answer is that pthreads and qthreads just cannot communicate ? – othane May 18 '11 at 23:25
1

You need a QEventLoop to process signal/slot stuff in the new thread.

QTimer needs those to work.

void *ProcessAsyncTasks(void */*ptr*/)
{    
    QEventLoop loop;
    StartTimers();    
    loop.exec();
    return NULL; 
}

Why not use QThread?

0xbaadf00d
  • 2,535
  • 2
  • 24
  • 46
  • I will give this a go but it is not really feasible for our final design as we get events from a external library of a pthread. I did try using the multitimers run method (because it is a thread) to call exec but that didnt work. – othane May 18 '11 at 23:05
  • I think that you should have another look at what the classes do and require to work and you will find a better answer. I have a hard time figuring out something that would help without knowing what it is that you need to achieve. – 0xbaadf00d May 19 '11 at 05:12
0

OK for anyone also trying to solve this, I think I figured this out, if I move the m_timer and multitimer objects back to the main qt thread and emit the timer start signal "properly" it all seems to work (see below).

Possibly I could still make a exec call in the qthread to take over from the pthread and move the multitimer and m_timer to that, but this way works for now and I get my "Got timered by timer %d.\n" output on time even after the pthread is dead as desired.:

Thanks all for the input, if there is a better way to do this or a massive mistake I have overlooked it would be good to know? Cheers

multitimer.h:

/**
    @file multitimer.h
    @brief Partial implementation of Windows-like SetTimer()/KillTimer() for Qt.

*/
#pragma once

#ifndef MULTI_TIMER_H
#define MULTI_TIMER_H

#include <QThread>
#include <QTimer>
#include <QMutex>

#include <QMap>


#include <QMetaType>
#include <cassert>


class MultiTimer : public QThread
{
    Q_OBJECT

public:
    typedef void (*multiTimerCallback)(quint32 p_id);

private:
    QTimer m_timer;
    QMutex m_mutex;
    quint32 m_id;
    multiTimerCallback m_callback;
    void KillTimer(void);

public:
    // only TimerFactory is allowed to instantiate MultiTimer
    MultiTimer(quint32 p_id, multiTimerCallback p_callback);
    ~MultiTimer();
    enum TTimerType
    {
        TT_SingleShot,      ///< Timer fires only once
        TT_Repetitive       ///< Timer keeps firing repeatedly until stopped with KillTimer()
    };
    void SetTimer(quint32 p_delayInMilliseconds, MultiTimer::TTimerType timerType);

private slots:
    void Update(void);

signals:
    void start_sig(int);
};

#endif    

timer.cpp:

#include <QtCore/QCoreApplication>
#include "multitimer.h"
#include <stdio.h>


//--------------------------------------------------------------------------------------------------

void MultiTimer::SetTimer(quint32 p_delayInMilliseconds, MultiTimer::TTimerType timerType)
{
    QMutexLocker locker(&m_mutex);
    m_timer.setSingleShot(TT_SingleShot == timerType ? true : false);
    connect(this, SIGNAL(start_sig(int)), &m_timer, SLOT(start(int)), Qt::QueuedConnection);
    //m_timer.start(p_delayInMilliseconds);
    emit start_sig(p_delayInMilliseconds);
    disconnect(this, SIGNAL(start_sig(int)), &m_timer, SLOT(start(int)));
}

void MultiTimer::KillTimer(void)
{
    QMutexLocker locker(&m_mutex);
    m_timer.stop();
}

void MultiTimer::Update(void)
{
    QMutexLocker locker(&m_mutex);

    if (NULL != m_callback)
        m_callback(m_id);
}

MultiTimer::MultiTimer(quint32 p_id, multiTimerCallback p_callback)
    : m_id(p_id)
    , m_callback(p_callback)
{
    bool isConnected = true;
    isConnected &= this->connect(&this->m_timer, SIGNAL(timeout()), this, SLOT(Update()), Qt::QueuedConnection);
    assert(isConnected);
    //this->start();

    moveToThread(qApp->thread());
    m_timer.moveToThread(qApp->thread());
}

MultiTimer::~MultiTimer()
{
    KillTimer();
    wait();
}


//--------------------------------------------------------------------------------------------------
#define ASYNC_TIMERS
#define xGLOBAL_TIMERS

void Callback(quint32 p_id)
{
    printf("Got timered by timer %d.\n", p_id);
}

MultiTimer *mt;
void StartTimers(void)
{
    #ifndef GLOBAL_TIMERS
    mt = new MultiTimer(1, Callback);
    #endif
    mt->SetTimer(2000, MultiTimer::TT_SingleShot);
}

#ifdef ASYNC_TIMERS
pthread_t AsyncTaskThread;
void *ProcessAsyncTasks(void */*ptr*/)
{
    StartTimers();
    return NULL;
}
#endif

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);

    #ifdef GLOBAL_TIMERS
    mt = new MultiTimer(1, Callback);
    #endif

    #ifdef ASYNC_TIMERS
    pthread_create(&AsyncTaskThread, NULL, &ProcessAsyncTasks, NULL);
    #else
    StartTimers();
    #endif

    return a.exec();
}
othane
  • 143
  • 1
  • 5
  • What are you trying to achieve? I cannot help you, unless I understand what it is that you want. If all you need is a QTimer that calls some function at some interval, you don't need to create threads at all for that. If you want the QTimers to run from different threads, you need QEventLoop to run in those threads. This is what you have in your main thread, since QCoreApplication provides it. – 0xbaadf00d May 23 '11 at 05:07