QTextDocument and Multithreading

Tyras

You parse your JSON in your worker object(s) (I'm assuming the low-level API here) and emit a signal from it on each ready block. The signal should have something like this as a signature

I can't really do it the way you suggested because the order of the lines must be preserved. What I tried to do at first was to preallocate a QVector which would contain all the lines, since I know the number of lines before parsing them, and pass the QVector and the index range to the worker thread.

The problem is, QTextBlockFormat and QTextFOrmat only stores the format rules - not actual text. The Class that stores the formatted text block is QTextBlock, but its objects are read-only.

kshegunov

@Tyras

I can't really do it the way you suggested because the order of the lines must be preserved.

How many workers do you have parsing a single JSON file?

Tyras

@kshegunov

How many workers do you have parsing a single JSON file?

the number of threads of the processor.

kshegunov

@Tyras
Then aggregate the block formats and texts in your own class (implicit sharing should work) and add an integer member for the line at which the parsing had started. Emit this object from your threads instead of the formats, and when the whole procedure has finished, only then you insert everything into the document. You could queue your objects in the builder for later processing if they're not the next pending text fragment. Does this make sense?

Alternatively build up text fragments label them for their order of occurrence and insert them in the slot building the document.

Kind regards.

Tyras

That... could actually work!

Gonna try as soon as I get time, and report back

Tyras

@kshegunov

The idea was very good, but It was impratical because I can (and usually do) have different formattings in a same line (for example, in a same line, some text is bold, some not). But it gave me a hint of how to implement it.

I found out that i can convert QTextDocument objects to QTextDocumentFragment objects, and then insert the latter into a new QTextDocument. So, now, In each thread, I create a QTextDocument, fill it with the text and send it to the the builder Thread.

void JsonParser::_workerFinished()
{
	if(workerPool.activeThreadCount())
		return;

	QTextDocument *doc = new QTextDocument();
	QTextCursor cursor(doc);

	while(!logFrags->isEmpty())
	{
		QTextDocument *fragDoc = logFrags->first()->clone(this);
		QTextDocumentFragment frag(fragDoc);

		cursor.insertFragment(frag);
		delete logFrags->takeFirst();
	}

	delete logFrags;

	emit processingFinished(doc);
}

It works, but the line

QTextDocument *fragDoc = logFrags->first()->clone(this);

outputs

QObject: Cannot create children for a parent that is in a different thread.
(Parent is QTextDocument(0x810cf2bd70), parent's thread is QThread(0x810ce03ec0), current thread is QThread(0x810923faa0)

I really don't understand why, since the line in question isn't suppose to write anything to the QTextDocument. And whats even more strange: It only happens in the loop's first iteration.

Any hints about what's happening?

*EDIT: My Bad, I just noticed you suggested using text fragments. Thanks!

kshegunov

@Tyras

I really don't understand why, since the line in question isn't suppose to write anything to the QTextDocument.

This usually indicates you have some mismatch between QObject instances, i.e. you're trying to create an object in one thread that has a parent having affinity for another. QObject object hierarchies must be living in the same thread, you can't have a parent that's in one thread and then a child in another. That's why you "push" your worker objects into another thread. One more thing to note is that you can "push" a QObject to another thread, provided he has no parent/children, but you can't "pull" it out of a thread. That's why I was suggesting to either use the formats, text and so on in your aggregated class or use text fragments, because none of those classes derive from QObject and can be passed around easily.

Kind regards.

Tyras

@kshegunov

you're trying to create an object in one thread that has a parent having affinity for another.

That's exactly what I don't understand. QTextDocument::clone() was supposed to be like a copy constructor: create a new object, read the cloned one's contents and write into the new one. It's supposed to create children for the new object, not the original one.

SGaist

Hi,

What you are currently doing is giving a parent to that new QTextDocument, you should rather not give the parent parameter and then move your cloned QTextDocument to your current thread with moveToThread.

kshegunov

@Tyras
I'm not quite sure what is where in your snippet, but @SGaist's comment looks to be on the right track, so I suggest following his advice.

Tyras

@SGaist

you should rather not give the parent parameter and then move your cloned QTextDocument to your current thread with moveToThread.

Should I just ignore the warning, and just move to the current thread, then? It's the clone method that gives the warning (confirmed it in debug).

kshegunov

@Tyras
No.
If logFrags is QVector<QTextDocument> then this:

logFrags->first()->clone(this)

Will parent it to your parser object, which lives in your parser thread. Assuming that vector comes from another thread, then it causes issues.
Something like this, should be working okay:

QTextDocument * fragDoc = logFrags->first()->clone();
fragDoc->moveToThread(QThread::currentThread());

Kind regards.

Tyras

@kshegunov
Just tried your code.

QTextDocument * fragDoc = logFrags->first()->clone();

outputs:

QObject: Cannot create children for a parent that is in a different thread.
(Parent is QTextDocument(0x4b9ae26640), parent's thread is QThread(0x4b9acaa630), current thread is QThread(0x4b97a007c0)

kshegunov

Yes, I'm missing on something it seems. Why would you want to clone the objects anyway, can you just push them into the current thread?

QTextDocument * fragDoc = logFrags->first();
fragDoc->moveToThread(QThread::currentThread());

Tyras

@kshegunov

Why would you want to clone the objects anyway, can you just push them into the current thread?

Because I need to push it in the thread that created it... but, well, since the target thread is a singleton, it won't be so ugly.

kshegunov

@Tyras

since the target thread is a singleton

I don't understand this. Do you mean to tell that the QThread object is a singleton?

Tyras

@kshegunov
Sorry, I meant that the instance of the class that receives the QTextDocument is a singleton

kshegunov

@Tyras
You shouldn't have singletons in the first place, much less QObject derived singletons.
That being said, you'll have to tell where are the instances in the logFrags vector (if it's a vector) created and where the JsonParser object is residing, and by where I mean in what thread.

Additionally my previous comment:

Yes, I'm missing on something it seems. Why would you want to clone the objects anyway, can you just push them into the current thread?

Is absolutely wrong, since I was suggesting you try to "pull" an object from another thread, which is not possible.

Kind regards.

Tyras

@kshegunov

You shouldn't have singletons in the first place, much less QObject derived singletons.

I don't really understand why, since the class is a controller, I should never have more than one instance of it in my application, And I should be able to access this single instance through all the code... besides, singletons are a design pattern... And it is QObject derivated because it must live in it's own thread.

But, that aside, I'll just have the worker threads push the QTextDocument objects to JsonParser threads after they finish them.

Thanks for the help!

kshegunov

@Tyras

I don't really understand why, since the class is a controller, I should never have more than one instance of it in my application, And I should be able to access this single instance through all the code... besides, singletons are a design pattern... And it is QObject derivated because it must live in it's own thread.

You shouldn't have singletons because:

A singleton is not a real object, it's a facade for a global variable
A singleton created on the stack is initialized before main, so anything that actually depends on things done in main() as QObject does, may or may not work.
A singleton that's constructed on the heap often is simply left undeleted - a memory leak. C++ is not JAVA, it's the programmers job to clean the memory up.
A singleton that's created on first use in the heap requires special measures to be taken, so the construction is thread safe.
A singleton created on the stack can't guarantee order of initialization (whence point 2 derives). If you have more than one the loader will initialize them depending on its mood!
A singleton that's created on the heap can't guarantee order of initialization ever!
A singleton is a global shared public resource in your application that promotes coupling, it actually couples every one of the classes that decide to use it.
A singleton is not thread-safe by design, and can't be reentrant as there is only one.

Why your worker object should not be a singleton:

Because you can have many worker objects in different threads.
Because if you want only one, you create only one! (And I just can't stress this enough)
Because the signal/slot mechanism is supposed to decouple your components, not the other way around.
Because object hierarchies work terribly with singletons, who's the root object is simply undefined.
The fact that something is called a "design pattern" in some book, doesn't mean you should use it.
Because something is possible, doesn't mean you should do it.

Have you seen singletons actually implemented in Qt? Yes, there are static functions, yes there are static variables (if you skim through the code) there's a lot of them actually. But have you seen a real life singleton in this enormously big library? I have not!

Simply apply some common sense! Create your nice worker object, connect its signals and slots, connect the cleanup routines and just rock its bloody world!

QObject::deleteLater is actually a slot and this is not at all a coincidence, neither is the QApplication::aboutToQuit signal a fling, nor is the QObject::destroyed(QObject *) signal an error.
My advice is: forget the "singleton design pattern" and enter the great world of OOP, yes a singleton breaks almost every rule in OOP.

Man, sometimes I just want to cry ... then I get angry, and then I surrender. I must be dying a little with each of these posts and I firmly believe I've given at least several of these impassioned preaches ...