Help with timer and JavaScript slot.

kshegunov

Hold your horses! You have managed to confuse me about what's modified and what goes in which function. In any case in the last piece of code; after this:

//Create new timer
qdbg() << "Before Timer creation: " << pobjCurrThread;
            ptmrScript = new QTimer(pobjCurrThread);
    //Register the new timer using the supplied ID

adding

Q_ASSERT(clsJSON::pobjGetScriptEng()->currentThread() == QThread::currentThread());

should get tripped.

PS.
This does nothing:

ptmrScript = new QTimer;
ptmrScript->moveToThread(pobjCurrThread);

SPlatten

@kshegunov said in Help with timer and JavaScript slot.:

Q_ASSERT(clsJSON::pobjGetScriptEng()->currentThread() == QThread::currentThread());

I cannot call currentThread() from pobjGetScriptEng(), it returns a pointer to QJSEngine.

kshegunov

@SPlatten said in Help with timer and JavaScript slot.:

I cannot call currentThread() from pobjGetScriptEng(), it returns a pointer to QJSEngine.

Correct. I meant to write:

Q_ASSERT(clsJSON::pobjGetScriptEng()->thread() == QThread::currentThread());

SPlatten

@kshegunov , realised that after posting, modified output to:

qdbg() << "Script Eng Thread: " << pobjScriptEng->thread();
qdbg() << "Before Timer creation: " << pobjCurrThread;

Result is:

S000000000028E000000000784T09:03:45.243DL00000642F../clsScriptHelper.cpp[void clsScriptHelper::createTimer]
Script Eng Thread: QThread(0x60000000c0d0)
S000000000029E000000000784T09:03:45.243DL00000643F../clsScriptHelper.cpp[void clsScriptHelper::createTimer]
Before Timer creation: QThread(0x60000026f260)
S000000000030E000000000784T09:03:45.243W:QObject: Cannot create children for a parent that is in a different thread.
(Parent is clsScriptHelper(0x600000208d80), parent's thread is QThread(0x60000000c0d0), current thread is QThread(0x60000026f260)
S000000000031E000000000784T09:03:45.244DL00000664F../clsScriptHelper.cpp[void clsScriptHelper::createTimer]
Before QObject::connect: QThread(0x60000026f260)

SPlatten

@kshegunov , have now changed code to:

ptmrScript = new QTimer(this);
ptmrScript->moveToThread(pobjScriptEngThread);
ptmrScript->setInterval(cuintInterval);

Output is now different:

setupForm() !!!!
S000000000028E000000000716T09:06:20.132W:QObject: Cannot create children for a parent that is in a different thread.
(Parent is clsScriptHelper(0x600000202820), parent's thread is QThread(0x600000004150), current thread is QThread(0x60000025e1c0)
S000000000029E000000000716T09:06:20.132W:QObject::startTimer: Timers cannot be started from another thread
S000000000030E000000000716T09:06:20.132DL00000688F../clsScriptHelper.cpp[void clsScriptHelper::createTimer]
Is timer active: true
S000000000031E000000000716T09:06:20.132DL00000207Fsimon2.js[void clsScriptHelper::log]
setupForm() !!!!

I will add another API function to allow the script to start the timer.

JonB

@SPlatten said in Help with timer and JavaScript slot.:

Cannot create children for a parent that is in a different thread.

You still have this. Excuse my complete guesswork, but do you need to go new QTimer(); rather than new QTimer(this); to move to another thread? (I have never been sure which statement causes the error message, which you can discover by stepping over or putting in qDebug() markers.)

But if all this is beyond me and being solved by @kshegunov please ignore me --- I'm just watching the discussion :)

kshegunov

Right, so now comes the million bucks question(s):

Why is clsScriptHelper::createTimer called from thread different from the one pobjScriptEng has affinity for?
Why do you need to use threads to begin with?

As for the "solution":

QMetaObject::Connection cn = QObject::connect(ptmrScript
                                                         ,&QTimer::timeout

Shall provide a context object for the execution. Looking at the body, I'd say it should be something like:

QMetaObject::Connection cn = QObject::connect(ptmrScript , &QTimer::timeout, pobjScriptEng
                                                         ,[pobjScriptEng
                                                          ,ptmrScript
                                                          ,strFile
                                                          ,strFunc
                                                          ,strScriptWithCall]() {

Also the timer should be created with the correct affinity, I assume:

ptmrScript = new QTimer(pobjScriptEng);

The original questions stand notwithstanding

PS: You have race conditions all over the place ...

SPlatten

@kshegunov , createTimer is called by JavaScript:

function setupForm() {
	//Query all people	  
	xmleng.log(0, "setupForm() !!!!", cstrThis, 202);
	//Create request counter property
	xmleng.setProperty(cstrFormTest, cstrStageCtr, "0");
	//Create timer to issue SQL requests
	xmleng.createTimer("t1", 50, "simon2.js@testTimer");
	xmleng.log(0, "setupForm() !!!!", cstrThis, 207);
	xmleng.startTimer("t1");
};

SPlatten

@kshegunov , I've modified my source to:

 QMetaObject::Connection cn = QObject::connect(ptmrScript
                                              ,&QTimer::timeout
                                              ,pobjScriptEng
                                              ,[pobjScriptEng
                                               ,ptmrScript
                                               ,strFile
                                               ,strFunc
                                               ,strScriptWithCall]() {

still getting:

S000000000027E000000000747T09:17:30.915DL00000202Fsimon2.js[void clsScriptHelper::log]
setupForm() !!!!
S000000000028E000000000747T09:17:30.915DL00000207Fsimon2.js[void clsScriptHelper::log]
setupForm() !!!!
S000000000029E000000000747T09:17:30.915W:QObject::startTimer: Timers cannot be started from another thread

kshegunov

@SPlatten said in Help with timer and JavaScript slot.:

still getting

Did you also parent the timer to the QJSEngine?

SPlatten

@kshegunov, this is how it is now:

QJSEngine* pobjScriptEng(clsJSON::pobjGetScriptEng());
QThread* pobjScriptEngThread(pobjScriptEng->thread());
//Create new timer
ptmrScript = new QTimer();
ptmrScript->moveToThread(pobjScriptEngThread);
ptmrScript->setInterval(cuintInterval);

Have just modified to:

QJSEngine* pobjScriptEng(clsJSON::pobjGetScriptEng());
QThread* pobjScriptEngThread(pobjScriptEng->thread());
//Create new timer
ptmrScript = new QTimer();
ptmrScript->setParent(pobjScriptEng);
ptmrScript->moveToThread(pobjScriptEngThread);
ptmrScript->setInterval(cuintInterval);

Output is still:

S000000000027E000000000743T09:24:17.520DL00000202Fsimon2.js[void clsScriptHelper::log]
setupForm() !!!!
S000000000028E000000000743T09:24:17.520W:QObject::setParent: Cannot set parent, new parent is in a different thread
S000000000029E000000000743T09:24:17.520DL00000207Fsimon2.js[void clsScriptHelper::log]
setupForm() !!!!

JonB

@SPlatten
Perhaps not relevant, but how can you go

            ptmrScript->moveToThread(pobjScriptEngThread);
            ptmrScript->setInterval(cuintInterval);

Once moved to other thread, are you still allowed to set its interval from this (different) thread?

kshegunov

Okay, I'm getting really tangled. Where is QTimer::start called? In createTimer? If so, then:

QTimer timer = new QTimer();
...
// Connect the lambda with a context object, as mentioned!
...
timer->start();

SPlatten

@kshegunov , I'm trying lots of things without taking the time to think about it.

kshegunov

@SPlatten said in Help with timer and JavaScript slot.:

I'm trying lots of things without taking the time to think about it.

Yes, do so. Which is the whole reason I requested the call stack. How am I supposed to guess what calls what ...

SPlatten

@kshegunov I would provide the call stack, but execution doesn't halt, so where would you like me to break to then get the stack?

KroMignon

@SPlatten I am always surprised how complicated you are trying to solve common issues.

What is the real issue here?
You want to create a timer in a specific thread and start it. I am right?
QTimer muss be started, and stopped from the owned thread.
That's not so complicated to solve:

auto myTimer = new  QTimer();
connect(myTimer; &QTimer::timeout, ...);
// move to thread
myTimer->moveToThread(theWorkingThread);

// start the timer
QTimer::singleShot(0, myTimer, [myTimer]() { myTimer->start(); } );

SPlatten

@KroMignon , thank you, will try this now, right now before trying your suggestion the output is:

S000000000027E000000000715T09:32:03.904DL00000202Fsimon2.js[void clsScriptHelper::log]
setupForm() !!!!
S000000000028E000000000715T09:32:03.904DL00000207Fsimon2.js[void clsScriptHelper::log]
setupForm() !!!!
S000000000029E000000003405T09:32:06.594W:QObject::startTimer: Timers cannot be started from another thread

@KroMignon, thank you so much, this now works, here are the prototypes:

Q_INVOKABLE void createTimer(const QString& crstrID, const uint cuintInterval
                            ,const QString& crstrFunction);
Q_INVOKABLE void startTimer(const QString& crstrID);

And the final implementations:

/**
 * @brief clsScriptHelper::createTimer
 * @param crstrID : Constant reference to Timer ID
 * @param cuintInterval : Constant millisecond interval
 * @param crstrFunction : Constant reference to JSfile@Function
 */
void clsScriptHelper::createTimer(const QString& crstrID, const uint cuintInterval
                                 ,const QString& crstrFunction) {
    if ( crstrID.trimmed().isEmpty() == true || cuintInterval == 0
      || crstrFunction.trimmed().isEmpty() == true
      || crstrFunction.indexOf(clsXMLnode::msccScriptDelimiter) == -1 ) {
    //One or more of the parameters are invalid, do nothing
        return;
    }
    //Get Application instance
    clsMainWnd* pAppInstance(clsMainWnd::pobjGetAppWnd());
    if ( pAppInstance == nullptr ) {
    //Cannot get application instance!
        return;
    }
    //Look for registered timer
    QTimer* ptmrScript(pAppInstance->ptmrGetTimer(crstrID));
    if ( ptmrScript != nullptr ) {
    //Already registered, do nothing
        return;
    }
    //No timer locatd with that ID, its safe to proceed
    QStringList slstScript = crstrFunction.split(clsXMLnode::msccScriptDelimiter);
    mpScripts* pmpScriptsMap;
    if ( slstScript.length() == SSF_PARAM_COUNT
    && (pmpScriptsMap = clsMainWnd::pmpGetScriptsMap()) != nullptr ) {
    //Look for the script in the scripts map
        mpScripts::iterator itrScript(pmpScriptsMap->find(slstScript[SFF_FILE_IDX]));
        if ( itrScript != pmpScriptsMap->end() ) {
    //Found it, build the script up ready for the signal connection
            QString strScript(itrScript->second);
    //Get pointer to script engine
            QJSEngine* pobjScriptEng(clsJSON::pobjGetScriptEng());
    //Create new timer
            ptmrScript = new QTimer();
            ptmrScript->setInterval(cuintInterval);
    //Register the new timer using the supplied ID
            pAppInstance->registerTimer(ptmrScript, crstrID);
    //Add any globals to the script
            clsScriptHelper::pobjGetInstance()->addGlobals(pobjScriptEng);
    //Build up script function call
            QString strFile(slstScript[SFF_FILE_IDX])
                   ,strCall(slstScript[SSF_FUNCTION_IDX])
                   ,strScriptWithCall(strScript)
                   ,strFunc(strCall
                          + QString(clsDebugService::msccBrktOpen)
                          + QString(clsDebugService::msccBrktClose)
                          + QString(clsXMLnode::msccScriptFunctionTerminator));
             if ( strScriptWithCall.endsWith(clsXMLnode::msccScriptFunctionTerminator) != true ) {
                 strScriptWithCall += clsXMLnode::msccScriptFunctionTerminator;
             }
             strScriptWithCall += strFunc;
    //Create connection to timer and script
             QMetaObject::Connection cn = QObject::connect(ptmrScript
                                                         ,&QTimer::timeout
                                                         ,pobjScriptEng
                                                         ,[pobjScriptEng
                                                          ,ptmrScript
                                                          ,strFile
                                                          ,strFunc
                                                          ,strScriptWithCall]() {
                 QJSValue objResult(pobjScriptEng->evaluate(strScriptWithCall));
                 QString strError;
                 if ( objResult.isError() == true ) {
                     strError = QString("%1\n%2\n%3").arg(strFile).arg(strFunc)
                                                     .arg(objResult.toString());
                 } else {
                     QString strResult(objResult.toString());
                     if ( strResult.compare("undefined") != 0 ) {
                         strError = strResult;
                     }
                 }
                 if ( strError.isEmpty() != true ) {
                     clsXMLnode* pobjRoot(clsXMLnode::spobjGetRoot());
                     emit pobjRoot->error(strError);
                     ptmrScript->deleteLater();
                 }
            });
            QThread* pobjScriptEngThread(pobjScriptEng->thread());
            ptmrScript->moveToThread(pobjScriptEngThread);
        }
    }
}
/**
 * @brief clsScriptHelper::startTimer
 * @param crstrID : Constant reference to timer ID
 */
void clsScriptHelper::startTimer(const QString& crstrID) {
    //Get Application instance
    clsMainWnd* pAppInstance(clsMainWnd::pobjGetAppWnd());
    if ( pAppInstance == nullptr ) {
    //Cannot get application instance!
        return;
    }
    QTimer* ptmrScript(pAppInstance->ptmrGetTimer(crstrID));
    if ( ptmrScript != nullptr ) {
        QTimer::singleShot(0, ptmrScript, [ptmrScript]() { ptmrScript->start(); } );
    }
}

kshegunov

@SPlatten said in Help with timer and JavaScript slot.:

I would provide the call stack, but execution doesn't halt, so where would you like me to break to then get the stack?

Before doing the connect for the timer timeout. Provide the following things (in one post please):

The whole source of the enclosing function.
The call stack.
The output of this added just before doing the connect:

qDebug() << ptmrScript->thread() << QThread::currentThread() << ptmrScript->parent();

KroMignon

@SPlatten said in Help with timer and JavaScript slot.:

right now before trying your suggestion the output is:

   setupForm() !!!!
   S000000000029E000000003405T09:32:06.594W:QObject::startTimer: Timers cannot be started from another thread

Same issue, you are trying to start a QTimer from the wrong thread.
QTimer::singleShot(0, myTimer, [myTimer]() { myTimer->start(); } ); will solve this.