Slot re-entrancy Windows7 using QT dlls

**MBoss131** · 16th March 2016, 18:12

We have a QSpinBox which is connected to a SLOTfor the valueChanged SIGNAL. When the user presses the up arrow, the valueChanged SLOT is called and processing begins. This is, of course, as desired.

However, while in the SLOT valueChanged we do our processing. When the processing takes too long, we get another valueChanged (but not from the user, from, we believe, inside QT). This causes us to process again (and the spin box value to increment). I will stop and mention at this point that we will be moving to a more multi-threaded approach, but we wanted to get our prototype done before we delved into that. I see now reason why this should not work as expected. an XCode build works as desired.

To further the issue, if QApplication::processEvents() is called after the long processing, then the valueChanged SLOT is called again; becoming re-entrant. I read an old posting: Preventing a Slot to be called again before it has finished running, which suggested that this was impossible whether asynchronous or synchronous, and/or was due to heap corruption. One poster pointed out some ideas, one with processEvents. With all that being said, I took to writing a simple application that exposed the issue I am having.

I won't call it a QT bug, but it sure shows the slot being called multiple times for a single user click of the up arrow. It also shows a re-entrancy issue when processEvents() is called and the user has clicked the arrow several times. Attached is the sample code. Now I will walk thru the sample code and the output.

My question is: Is this a bug or something we are not realizing about slots in a single threaded QT application. Code and output follows.

Qt Code:

Switch view

#include "mainwindow.h"
 
#include <QDebug>
 
#include <thread>
#include <chrono>
 
// set to connect/disconnect the slot
#define CONNECT_DISCONNECT_FIX 0
// set PROCESS_EVENTS to 0 to not issue QApplication::processEvents();
#define PROCESS_EVENTS		0
// set SLEEP_MILLISECONDS to how long to delay in slot (< 350 seems to work, 2000 is usually guaranteed
#define SLEEP_MILLISECONDS	2000
 
// show some output
#define DEBUG_OUTPUT(strDescription,bInSlot,nID) qDebug() << (strDescription) << "\n\tID: " << (nID) << "\n\tInSlot: "<< (bInSlot) << "\n\tThreadID: " << _threadid << "\n\tTimeStamp: " << std::chrono::high_resolution_clock::now().time_since_epoch().count() << "\n\n";
 
MainWindow::MainWindow(QWidget* parent) :
	QMainWindow(parent)
{
	m_UI.setupUi(this);
 
	bool bResult = connect(m_UI.spinBox,SIGNAL(valueChanged(int)),this,SLOT(OnSlotSpinBox(int)),Qt::QueuedConnection);
}
 
MainWindow::~MainWindow()
{
}
 
void MainWindow::OnSlotSpinBox(int nValue)
{
	#if CONNECT_DISCONNECT_FIX
	bool bResult = disconnect(m_UI.spinBox,SIGNAL(valueChanged(int)),this,SLOT(OnSlotSpinBox(int)));
	#endif
 
	// unique ID
	static uint32_t snID = 0;
 
	// transfer to local, so it is right when re-entrancy occurs
	uint32_t nID = ++snID;
 
	static bool sbInSlot = false;
	DEBUG_OUTPUT("[enter] OnSlotSpinBox",sbInSlot,nID);
 
	if (!sbInSlot) {
		sbInSlot = true;
 
		DEBUG_OUTPUT("[sleep-start] OnSlotSpinBox",sbInSlot,nID);
		std::this_thread::sleep_for(std::chrono::milliseconds(SLEEP_MILLISECONDS));
 
		#if PROCESS_EVENTS
		DEBUG_OUTPUT("[processEvents-start] OnSlotSpinBox",sbInSlot,nID);
		QApplication::processEvents();
		DEBUG_OUTPUT("[processEvents-end] OnSlotSpinBox",sbInSlot,nID);
		#endif
 
		DEBUG_OUTPUT("[sleep-end] OnSlotSpinBox",sbInSlot,nID);
 
		sbInSlot = false;
	}
	else {
		DEBUG_OUTPUT("[RE-ENTRANCY DETECTED] OnSlotSpinBox",sbInSlot,nID);
	}
 
	DEBUG_OUTPUT("[exit] OnSlotSpinBox",sbInSlot,nID);
 
	#if CONNECT_DISCONNECT_FIX
	bResult = connect(m_UI.spinBox,SIGNAL(valueChanged(int)),this,SLOT(OnSlotSpinBox(int)),Qt::QueuedConnection);
	#endif
}

#include "mainwindow.h"

#include <QDebug>

#include <thread>
#include <chrono>

// set to connect/disconnect the slot
#define CONNECT_DISCONNECT_FIX 0
// set PROCESS_EVENTS to 0 to not issue QApplication::processEvents();
#define PROCESS_EVENTS		0
// set SLEEP_MILLISECONDS to how long to delay in slot (< 350 seems to work, 2000 is usually guaranteed
#define SLEEP_MILLISECONDS	2000

// show some output
#define DEBUG_OUTPUT(strDescription,bInSlot,nID) qDebug() << (strDescription) << "\n\tID: " << (nID) << "\n\tInSlot: "<< (bInSlot) << "\n\tThreadID: " << _threadid << "\n\tTimeStamp: " << std::chrono::high_resolution_clock::now().time_since_epoch().count() << "\n\n";

MainWindow::MainWindow(QWidget* parent) :
	QMainWindow(parent)
{
	m_UI.setupUi(this);

	bool bResult = connect(m_UI.spinBox,SIGNAL(valueChanged(int)),this,SLOT(OnSlotSpinBox(int)),Qt::QueuedConnection);
}

MainWindow::~MainWindow()
{
}

void MainWindow::OnSlotSpinBox(int nValue)
{
	#if CONNECT_DISCONNECT_FIX
	bool bResult = disconnect(m_UI.spinBox,SIGNAL(valueChanged(int)),this,SLOT(OnSlotSpinBox(int)));
	#endif

	// unique ID
	static uint32_t snID = 0;

	// transfer to local, so it is right when re-entrancy occurs
	uint32_t nID = ++snID;

	static bool sbInSlot = false;
	DEBUG_OUTPUT("[enter] OnSlotSpinBox",sbInSlot,nID);

	if (!sbInSlot) {
		sbInSlot = true;

		DEBUG_OUTPUT("[sleep-start] OnSlotSpinBox",sbInSlot,nID);
		std::this_thread::sleep_for(std::chrono::milliseconds(SLEEP_MILLISECONDS));

		#if PROCESS_EVENTS
		DEBUG_OUTPUT("[processEvents-start] OnSlotSpinBox",sbInSlot,nID);
		QApplication::processEvents();
		DEBUG_OUTPUT("[processEvents-end] OnSlotSpinBox",sbInSlot,nID);
		#endif

		DEBUG_OUTPUT("[sleep-end] OnSlotSpinBox",sbInSlot,nID);

		sbInSlot = false;
	}
	else {
		DEBUG_OUTPUT("[RE-ENTRANCY DETECTED] OnSlotSpinBox",sbInSlot,nID);
	}

	DEBUG_OUTPUT("[exit] OnSlotSpinBox",sbInSlot,nID);

	#if CONNECT_DISCONNECT_FIX
	bResult = connect(m_UI.spinBox,SIGNAL(valueChanged(int)),this,SLOT(OnSlotSpinBox(int)),Qt::QueuedConnection);
	#endif
}

To copy to clipboard, switch view to plain text mode

Scenario #1. Select up arrow once. (no processEvents() or connect/disconnect). The output clearly shows that the SLOT was called twice in a row.
[enter] OnSlotSpinBox
ID: 1
InSlot: false
ThreadID: 14080
TimeStamp: 14581510183319621
[sleep-start] OnSlotSpinBox
ID: 1
InSlot: true
ThreadID: 14080
TimeStamp: 14581510183319621
[sleep-end] OnSlotSpinBox
ID: 1
InSlot: true
ThreadID: 14080
TimeStamp: 14581510203443750
[exit] OnSlotSpinBox
ID: 1
InSlot: false
ThreadID: 14080
TimeStamp: 14581510203443750
[enter] OnSlotSpinBox
ID: 2
InSlot: false
ThreadID: 14080
TimeStamp: 14581510203443750
[sleep-start] OnSlotSpinBox
ID: 2
InSlot: true
ThreadID: 14080
TimeStamp: 14581510203443750
[sleep-end] OnSlotSpinBox
ID: 2
InSlot: true
ThreadID: 14080
TimeStamp: 14581510223567879
[exit] OnSlotSpinBox
ID: 2
InSlot: false
ThreadID: 14080
TimeStamp: 14581510223567879

Scenario #2. Select up arrow five times as fast as possible. (processEvents() and no connect/disconnect). The output clearly shows that the SLOT was called while in the slot (re-entrancy).
[enter] OnSlotSpinBox
ID: 1
InSlot: false
ThreadID: 16500
TimeStamp: 14581513537474084
[sleep-start] OnSlotSpinBox
ID: 1
InSlot: true
ThreadID: 16500
TimeStamp: 14581513537474084
[processEvents-start] OnSlotSpinBox
ID: 1
InSlot: true
ThreadID: 16500
TimeStamp: 14581513557598213
[processEvents-end] OnSlotSpinBox
ID: 1
InSlot: true
ThreadID: 16500
TimeStamp: 14581513557598213
[sleep-end] OnSlotSpinBox
ID: 1
InSlot: true
ThreadID: 16500
TimeStamp: 14581513557598213
[exit] OnSlotSpinBox
ID: 1
InSlot: false
ThreadID: 16500
TimeStamp: 14581513557598213
[enter] OnSlotSpinBox
ID: 2
InSlot: false
ThreadID: 16500
TimeStamp: 14581513557598213
[sleep-start] OnSlotSpinBox
ID: 2
InSlot: true
ThreadID: 16500
TimeStamp: 14581513557598213
[processEvents-start] OnSlotSpinBox
ID: 2
InSlot: true
ThreadID: 16500
TimeStamp: 14581513577722342
[enter] OnSlotSpinBox
ID: 3
InSlot: true
ThreadID: 16500
TimeStamp: 14581513577722342
[RE-ENTRANCY DETECTED] OnSlotSpinBox
ID: 3
InSlot: true
ThreadID: 16500
TimeStamp: 14581513577722342
[exit] OnSlotSpinBox
ID: 3
InSlot: true
ThreadID: 16500
TimeStamp: 14581513577722342
[enter] OnSlotSpinBox
ID: 4
InSlot: true
ThreadID: 16500
TimeStamp: 14581513577722342
[RE-ENTRANCY DETECTED] OnSlotSpinBox
ID: 4
InSlot: true
ThreadID: 16500
TimeStamp: 14581513577722342
[exit] OnSlotSpinBox
ID: 4
InSlot: true
ThreadID: 16500
TimeStamp: 14581513577722342
[enter] OnSlotSpinBox
ID: 5
InSlot: true
ThreadID: 16500
TimeStamp: 14581513577722342
[RE-ENTRANCY DETECTED] OnSlotSpinBox
ID: 5
InSlot: true
ThreadID: 16500
TimeStamp: 14581513577722342
[exit] OnSlotSpinBox
ID: 5
InSlot: true
ThreadID: 16500
TimeStamp: 14581513577722342
[processEvents-end] OnSlotSpinBox
ID: 2
InSlot: true
ThreadID: 16500
TimeStamp: 14581513577722342
[sleep-end] OnSlotSpinBox
ID: 2
InSlot: true
ThreadID: 16500
TimeStamp: 14581513577722342
[exit] OnSlotSpinBox
ID: 2
InSlot: false
ThreadID: 16500
TimeStamp: 14581513577722342

**anda_skoa** · 16th March 2016, 20:29

When you do processEvents() then Qt's event loop will process events, such as user interaction.
If the user interaction results in a value change of the spinbox, it will invoke the slot again.
So don't do that if you want to avoid artificial reentrancy.

Also: why do you make a queued connection for within the same thread?
If you leave it an auto connection, it will be a direct connection and the slot will be called immediately on the signal.
Currently you can queue up several signal emits.

Also: I don't see where in your first example you have reentrancy.
There is
enter -> sleep start -> sleep end -> exit for ID 1
then again for ID 2

So invocation for 1 has ended before 2 starts.

Cheers,
_

**MBoss131** · 16th March 2016, 20:45

Thank you for your reply. The original questions still remains unanswered. Why is my slot called twice when do long processing? There is only 1 connect and the user has only clicked once.

In response to your questions, here are my answers. I hope it helps.

You wrote...

When you do processEvents() then Qt's event loop will process events, such as user interaction.
If the user interaction results in a value change of the spinbox, it will invoke the slot again.
So don't do that if you want to avoid artificial reentrancy.

I understand processEvents() processes events, however, this is event should not be "pending" as it is already being handled? Is that not correct? It would seem to me that if processEvents() recalled all events that were in process then there would even be a bigger problem. There was only 1 user interaction which caused the original slot call.

You wrote...

Also: why do you make a queued connection for within the same thread?
If you leave it an auto connection, it will be a direct connection and the slot will be called immediately on the signal.
Currently you can queue up several signal emits.

None of the ConnectionType enum's change anything. That was put there as one of the attempts at fixing it.

You wrote...

Also: I don't see where in your first example you have reentrancy.
There is
enter -> sleep start -> sleep end -> exit for ID 1
then again for ID 2

Correct. Scenario #1 does not call processEvents(), therefore there is no re-entrancy. However, there are two calls to the slot, in response to 1 user click.

Further clarification:

1. Without processEvents(), the call happens twice when a long processing occurs. Yes, the calls are non-rentrant and can be considered queued. However, there is still only 1 user click; so I would not expect two slot calls.
2. With processEvents(), the call happens twice and the second call happens while the slot is running (i.e. re-entrancy).

note: the connect is only called once, so this idea of multiple connects (as stated in other threads) does not apply.

**anda_skoa** · 16th March 2016, 22:31

Is the value for both invocations the same?
Is there more than one connection?

Cheers,
_

**MBoss131** · 16th March 2016, 23:38

Thanks again for your reply.

Please see the attached code. The outputs are from two scenarios/tests. The first one does NOT call processEvents(). The second one does.

There is only one connection and it is called once. If you run the code in Windows 7 you will see the problem easily,

This is not an issue of multiple calls to connect.

As I debug further, I can extrapolate that that because the sleep takes too long, the signal is resent/requeued. If processEvents() (scenario #2) is called after the timeout, then the signal is processed inline. I would suggest that the QT signal processor is not removing the signal before it is processed. Therefore, in scenario #2 it is processed again (as it is still pending).

**anda_skoa** · 17th March 2016, 08:56

A normally connected signal is a direct method invocation in the single threaded case, so nothing is "sent".

So assuming you've fixed your connection mess, it would have to be multiple signal emits.

QSpinBox should only emit its valueChanged() signal when the value changed, hence my question if you are getting two different values.

Cheers,
_