| Trolltech Home | Qt-interest Home | Recent Threads | All Threads | Author | Date | |
| All threads index page 1 | |
1. I have some multi-threaded QTcpServer based code, which is
structurally similar to the the multi-threaded Fortuneserver
example.
2. My QTcpServer subclass creates a new thread in incomingConnection
like this:
> void IPCServer::incomingConnection(int socketd)
> {
> mdp_log(QObject::tr("received incoming connection"));
>
> IPCReader* reader = new IPCReader(socketd, parent_runnable_, this);
>
> connect(reader, SIGNAL(finished()), reader, SLOT(deleteLater()));
>
> mdp_log(QObject::tr("starting IPCReader thread"));
>
> reader->start();
> }
3. IPCReader is a subclass of QThread with the important code
looking like this:
> IPCReader::IPCReader(int socketd,
> Runnable* dest,
> QObject* parent)
> : QThread(parent),
> our_socketd_(socketd),
> destination_(dest),
> message_length_(0)
> {
> readsock_ = new QTcpSocket(this);
> readsock_->setSocketDescriptor(our_socketd_);
>
> connect(readsock_, SIGNAL( readyRead() ), this, SLOT( read_client() ));
> connect(readsock_, SIGNAL( disconnected() ), this, SLOT( deleteLater() ));
> }
>
> void IPCReader::run()
> {
> mdp_log(QObject::tr("starting IPCReader event loop"));
>
> exec();
> }
>
> void IPCReader::read_client()
> {
> mdp_log(QObject::tr("reading from client"));
> QDataStream stream(readsock_);
> ...
> }
3. When I connect to the QTcpServer, I see the log messages
from incomingConnection(), and immediately afterwards, I get
"QThread: Destroyed while thread is still running". I do not see
the log message from IPCReader::run() or from IPCReader::read_client().
Can anyone see what is wrong with this ? It seems that the thread dies
prior to reaching its run() method, but I can't see why.
--
[ signature omitted ]
On Monday 03 March 2008 13:20:33 Stephen Collyer wrote:
> 1. I have some multi-threaded QTcpServer based code, which is
> structurally similar to the the multi-threaded Fortuneserver
> example.
>
> 2. My QTcpServer subclass creates a new thread in incomingConnection
>
> like this:
> > void IPCServer::incomingConnection(int socketd)
> > {
> > mdp_log(QObject::tr("received incoming connection"));
> >
> > IPCReader* reader = new IPCReader(socketd, parent_runnable_, this);
> >
> > connect(reader, SIGNAL(finished()), reader, SLOT(deleteLater()));
> >
> > mdp_log(QObject::tr("starting IPCReader thread"));
> >
> > reader->start();
> > }
>
> 3. IPCReader is a subclass of QThread with the important code
>
> looking like this:
> > IPCReader::IPCReader(int socketd,
> > Runnable* dest,
> > QObject* parent)
> >
> > : QThread(parent),
> >
> > our_socketd_(socketd),
> > destination_(dest),
> > message_length_(0)
> > {
> > readsock_ = new QTcpSocket(this);
> > readsock_->setSocketDescriptor(our_socketd_);
> >
> > connect(readsock_, SIGNAL( readyRead() ), this, SLOT(
> > read_client() )); connect(readsock_, SIGNAL( disconnected() ), this,
> > SLOT( deleteLater() )); }
> >
> > void IPCReader::run()
> > {
> > mdp_log(QObject::tr("starting IPCReader event loop"));
> >
> > exec();
> > }
> >
> > void IPCReader::read_client()
> > {
> > mdp_log(QObject::tr("reading from client"));
> > QDataStream stream(readsock_);
> > ...
> > }
>
> 3. When I connect to the QTcpServer, I see the log messages
> from incomingConnection(), and immediately afterwards, I get
> "QThread: Destroyed while thread is still running". I do not see
> the log message from IPCReader::run() or from IPCReader::read_client().
>
> Can anyone see what is wrong with this ? It seems that the thread dies
> prior to reaching its run() method, but I can't see why.
I don't see the need for a thread. Make your IPCReader class derive from
QObject, remove the run() function and the call to start(). It should work.
Note that your QTcpSocket class is created on the main thread. It belongs to
that thread. And so does your IPCReader class. That means they signals are
delivered directly (not queued), but in the main thread.
Your run() function calls exec(), but that doesn't do anything, ever. It just
stays there, idling.
--
[ signature omitted ]
Attachment:
signature.asc
Description: This is a digitally signed message part.
Thiago Macieira wrote: > I don't see the need for a thread. Make your IPCReader class derive from > QObject, remove the run() function and the call to start(). It should work. I'm not sure what you mean by this - I'm taking the thread creation logic from the threaded fortune server example, in order to create a multi- threaded TCP server. If IPCReader class is not a QThread, then it'll be a single threaded server. Am I misunderstanding which thread you are referring to ? > Note that your QTcpSocket class is created on the main thread. It belongs to > that thread. And so does your IPCReader class. That means they signals are > delivered directly (not queued), but in the main thread. I'm not sure I follow this. The QTcpSocket readsock_ is a member of IPCReader, which is a QThread itself. > Your run() function calls exec(), but that doesn't do anything, ever. It just > stays there, idling. Yes. The problem here is that the threaded fortune server example does all of its useful work in the run() method, but I wanted to read from the connected socket via the readyRead() signal, in the associated slot read_client(). It then wasn't clear to me what my run() method should do, although it seemed like a bad idea to return immediately, for obvious reasons. -- [ signature omitted ]
On Monday 03 March 2008 14:44:55 Stephen Collyer wrote: > > I don't see the need for a thread. Make your IPCReader class derive from > > QObject, remove the run() function and the call to start(). It should > > work. > > I'm not sure what you mean by this - I'm taking the thread creation > logic from the threaded fortune server example, in order to create > a multi- threaded TCP server. If IPCReader class is not a QThread, > then it'll be a single threaded server. Am I misunderstanding which > thread you are referring to ? No. I'm saying you don't need it to be multi-threaded at all. In fact, you even designed it without the need to be multi-threaded. Your only mistake was to use QThread and start it. That's why you're getting the error. If you don't start the thread, the problem will be gone. > > Note that your QTcpSocket class is created on the main thread. It belongs > > to that thread. And so does your IPCReader class. That means they signals > > are delivered directly (not queued), but in the main thread. > > I'm not sure I follow this. The QTcpSocket readsock_ is a member of > IPCReader, which is a QThread itself. Common mistake. The QThread object does not belong to the thread it creates. It belongs to the thread that created it. It's also a very common mistake to initialise objects in the constructor. Remember that the constructor is run on the main thread, which means all objects created there belong to the main thread. > > Your run() function calls exec(), but that doesn't do anything, ever. It > > just stays there, idling. > > Yes. The problem here is that the threaded fortune server > example does all of its useful work in the run() method, > but I wanted to read from the connected socket via the readyRead() > signal, in the associated slot read_client(). It then wasn't clear > to me what my run() method should do, although it seemed like > a bad idea to return immediately, for obvious reasons. And that's a very good design. It doesn't need threads. -- [ signature omitted ]
Attachment:
signature.asc
Description: This is a digitally signed message part.
Thiago Macieira wrote: >> I'm not sure what you mean by this - I'm taking the thread creation >> logic from the threaded fortune server example, in order to create >> a multi- threaded TCP server. If IPCReader class is not a QThread, >> then it'll be a single threaded server. Am I misunderstanding which >> thread you are referring to ? > > No. > > I'm saying you don't need it to be multi-threaded at all. In fact, you even > designed it without the need to be multi-threaded. Your only mistake was to > use QThread and start it. That's why you're getting the error. When you say, "you even designed it without the need to be multi-threaded", are you referring to the use of the readyRead() signal ? If so, it's not clear to me why it need not be multi-threaded. Are you saying that I can follow the design of the TripServer in the Blanchette-Summerfield book, and still handle multiple non-blocking connections simultaneously ? (with the signal-slot mechanism multiplexing data to the appropriate readyRead() slot ?) The reason I added the additional thread is that at the end of the TripServer design, it says: "The server implementation that we have used doesn't scale very well when there are lots of connections. The problem is that while we are processing a request, we don't handle the other connections". I read that as implying that the server blocked for the duration of a single connection, but now I'm wondering if it actually means that multiple connections are multiplexed correctly, but inneficiently compared to a true multi-threaded implementation. > Common mistake. The QThread object does not belong to the thread it creates. > It belongs to the thread that created it. > > It's also a very common mistake to initialise objects in the constructor. > Remember that the constructor is run on the main thread, which means all > objects created there belong to the main thread. Yes, I need to look at this part of the code again. > And that's a very good design. > > It doesn't need threads. For the reasons I suggest above ? -- [ signature omitted ]
On Monday 03 March 2008 16:02:16 Stephen Collyer wrote: > Thiago Macieira wrote: > >> I'm not sure what you mean by this - I'm taking the thread creation > >> logic from the threaded fortune server example, in order to create > >> a multi- threaded TCP server. If IPCReader class is not a QThread, > >> then it'll be a single threaded server. Am I misunderstanding which > >> thread you are referring to ? > > > > No. > > > > I'm saying you don't need it to be multi-threaded at all. In fact, you > > even designed it without the need to be multi-threaded. Your only mistake > > was to use QThread and start it. That's why you're getting the error. > > When you say, "you even designed it without the need to be > multi-threaded", are you referring to the use of the readyRead() > signal ? Yes. > If so, it's not clear to me why it need not be multi-threaded. Let's be clear: "need not be" is not "must not be". It just means the absence of need. Your code can be threaded or not. You designed it that way by using the readyRead() signal, so that it acts when new data arrives, instead of blocking with the waitForReadyRead() function. If you don't need threading, I suggest you don't use it. It makes your life a lot simpler. > Are you saying that I can follow the design of the TripServer in > the Blanchette-Summerfield book, and still handle multiple non-blocking > connections simultaneously ? (with the signal-slot mechanism > multiplexing data to the appropriate readyRead() slot ?) I don't have the book ready to confirm what example you mean. So I can't answer the question. But you can handle multiple connections simultaneously from the same thread, including the main/GUI thread. That was the only way in Qt 1, 2 and 3 times (so, for 11 years of Qt's history). Introducing a new way in Qt 4 doesn't mean the old one doesn't work. > The reason I added the additional thread is that at the end > of the TripServer design, it says: "The server implementation > that we have used doesn't scale very well when there are lots > of connections. The problem is that while we are processing > a request, we don't handle the other connections". If you have that high a load, then you could consider using threads. But that number is very high. I suggest you test under real conditions first to determine if you need the use of threads. Also note that you can handle the sockets from the main thread, but leave the actual brunt of the work (if there's any) on the data received from the socket to other threads. It would be a simple case of producer-consumer threading, thus simplifying your code. If you do need to handle sockets in separate threads, I suggest keeping the TCP server on the main thread. Once you accept a new socket with nextPendingConnection(), you can call moveToThread() on it and place it on the appropriate handler thread. I see no need to keep the server in a separate thread. Handling incoming connections almost doesn't consume resources. (Before you receive more connections per second than you can handle, you will probably have hit other issues) > I read that as implying that the server blocked for the duration > of a single connection, but now I'm wondering if it actually > means that multiple connections are multiplexed correctly, but > inneficiently compared to a true multi-threaded implementation. There's no blocking in the Qt I/O classes unless you explicitly ask for it, with the waitFor* family of functions. Including the QTcpServer::waitForNewConnection() function. > > And that's a very good design. > > > > It doesn't need threads. > > For the reasons I suggest above ? Yes. -- [ signature omitted ]
Attachment:
signature.asc
Description: This is a digitally signed message part.
Thiago Macieira wrote: > But you can handle multiple connections simultaneously from the same thread, > including the main/GUI thread. That was the only way in Qt 1, 2 and 3 times > (so, for 11 years of Qt's history). OK, good point. > If you have that high a load, then you could consider using threads. But that > number is very high. I suggest you test under real conditions first to > determine if you need the use of threads. I can be pretty certain that I won't need threads for performance reasons. > Also note that you can handle the sockets from the main thread, but leave the > actual brunt of the work (if there's any) on the data received from the > socket to other threads. It would be a simple case of producer-consumer > threading, thus simplifying your code. In fact, my situation is reversed. The code I posted is already running in a sub-thread, collecting IPC messages, and delivering them to the message queue of its parent. So it's all producer-consumer threads anyway, but with the child thread handling network I/O not the parent. > I see no need to keep the server in a separate thread. Handling incoming > connections almost doesn't consume resources. (Before you receive more > connections per second than you can handle, you will probably have hit other > issues) My TCPServer will be running in a separate thread anyway for other design reasons, so I think that reduces the chance of any potential efficiency problem still further. OK. Thanks a lot. I now understand what's going on. I still have some outstanding questions about how a threaded design would actually work, but those can wait (for instance, if I use readyRead(), what do I need to put in my run() method ? - which was my original problem) -- [ signature omitted ]