PDA

View Full Version : No context switch between Threads and parent ?



donglebob
24th October 2008, 14:50
Hi,

The following situation is given:

There is an Object Instance of a Serial Class , which has the abilitiy to read() and write() on com-port. The Serial Class itself has in its privates an instance of a communication library I use.
The read() write() methods above use methods of this com. library.



class Serial:

Serial()
{
...
reader = new threads(this,true);
writer = new threads(this,false);

reader->start();
writer->start();
...
}

read()
{
//use here the com library function and some winapi functions to read from the com port
}

write()
{
// same as above but or writing
}

private:
Seriallibrary Slib; // instance of foreign serial library
Thread-Class *reader,*writer;



What I wanted to do is following:
Create two threads, one for reading operations and one for writing operations. (asynchron communication)
They had to use the methods I provide with my Serial Class.

What I have done is:

I created a "Thread-Class" derived from QThread.
I put two pointers of Type "Thread-Class" to my Serial Class (private member).
The Ctor of the "Thread-Class" has a boolean, to decide if it is a reader or a writer thread.
In dependency of that boolean one of two possible loops will be started.

These two loops use the Serial Class functions provided for writing or reading (with pointer serialPTR).
I think the whole stuff I designed is wrong. Is every function call I make from the threads running in the context of my Serial Class-Instance ????



Threads:

Threads(parent *p,bool typeofThread)
{
//take the parent pointer (serialPTR), because I need it to call the provided functions for reading and writing

//set a local flag if I am a Reader Thread or a Writer Thread.
}


run()
{
if(readerFlagIsSet)
while(!threadDone)
{
serialPTR->read();
}

if(writerFlagIsSet)
while(!threadDone)
{
serialPTR->write();
}

}
// I mean the two function calls above from the thread to the serial-Class



Can you give me a hint to make that better?
Or is this way ok?

wysota
24th October 2008, 18:10
Could we see the exact compilable code instead of that pseudocode? And also add the invocation code.

donglebob
26th October 2008, 18:52
Thx for answering.

Sorry I can just add pseudocode.
Maybe I could explain sth better, when it is not so clear..

wysota
26th October 2008, 19:06
I'm interested in proper declarations (prototypes) of objects and methods and the way you create and start the thread. You can skip any content which you do not want to expose.

donglebob
27th October 2008, 11:47
Hi no problem,

this is my thread-class:


#ifndef THREADS_H
#define THREADS_H

#include <QThread>


class serial; //forward decl.
class threads : public QThread
{
Q_OBJECT

public:
threads(QObject *parent,bool); //here i push the this-pointer of my serial class, so my Threads can call serial-class methods
~threads();
threads(threads &);
void stopAll(); // to stop my threads static var is set
protected:
void run();

private:
bool readerFlagIsSet,writerFlagIsSet; //ctor will set what kind of thread it is
static bool threadDone;
serial *serialPtr; // pointer to serial class object
};
#endif // THREADS_H

this is my serial class:



#ifndef SERIAL_H
#define SERIAL_H

#include <QObject>
#include <QString>
#include <QByteArray>
#include <QList>
#include <QMutex>
#include "define.h"
#include "storage.h"
#include <windows.h>
#include "threads.h" // my thread class (above)
#include "Serial\Serial.h" // this is the foreign serial lib I use


class serial : public QObject
{
Q_OBJECT

public:
serial(QObject *parent);
~serial();
serial(serial&);
bool openserialport(QString);
bool closeserialport();
QList<QString> scanSerialPorts();
void ResetEvents();

//threads fct

bool read(); //called by reader-thread in its loop.
//inside the read fct it waits for com events (WaitForMultipleObjects (Win32)) and blocks until data is there
bool isOpen();
bool isWriteJobAvailable(); //called by writer thread, to check if there is data in outgoing queue (to write)
void writeNewJob();// called by writer thread, to get data from queue and call write(QByteArray)
bool write(QByteArray); //called indirectly by writer thread to write data on device


signals:
void dataIN(); //emit signal when data is read in reader-thread, to inform another thread

private:
CSerial cserial; //foreign serial lib I use to access COM port
threads *readerThread,*writerThread; // pointer of type Threads
QByteArray *readBuf;
bool fContinue;
bool portIsOpen;

};

#endif // SERIAL_H


In my ctor of my serial class i create two instances, one for readerThread and one for writerThread and start the Threads.

Ctor of Serial Class:


reader = new threads(this,true);
writer = new threads(this,false);

reader->start();
writer->start();

run() of Threads:


void threads::run()
{
if(isReaderThread)
while(!threadDone){ //static var which will be set to stop the threads
serialPtr->read(); //serial class fct called with the pointer (got it inside from ctor)
msleep(50);
}

if(isWriterThread)
while(!threadDone){
if(serialPtr->isWriteJobAvailable()) //serial class fct called with the pointer (got it inside from ctor)
serialPtr->writeNewJob();
msleep(50);
}
}



Thanks!

wysota
28th October 2008, 00:21
I can say one thing - your multi-thread design is full of flaws :) First of all you access the same object from within more than one thread which is a Bad Thing (TM). Second of all all events for the serial object of yours will be processed in the main thread. I don't know if you actually use events there but based on what you observe, it is highly possible.

My first and most basic question is - are you sure you really need separate threads for the communication?

donglebob
28th October 2008, 09:24
First of all you access the same object from within more than one thread which is a Bad Thing

Maybe I should swap the design. The Threads will get the parent and the serial-classs will be shared between both.
I must use two threads because of two way communication (overlapped/asynchrone).
I think that it is common to do it like that in rs232 communication.

Here you see an example of MS
http://msdn.microsoft.com/en-us/library/ms810467.aspx

......a user interface thread that does memory management, a writer thread that controls all writing, and a reader/status thread that reads data and handles status changes on the port.



Second of all all events for the serial object of yours will be processed in the main thread. I don't know if you actually use events there but based on what you observe, it is highly possible

I worried about that. I think swapping would solve this?
All the access from the threads to the serial-class would be done in the thread context.

The threads must share just one thing, tha object of the foreign serial library, because this includes all stuff to access com-port.
Btw, i use this
http://www.codeproject.com/KB/system/serial.aspx

wysota
28th October 2008, 09:30
I must use two threads because of two way communication (overlapped/asynchrone).
If you use non-blocking IO, you don't have to have any separate threads.


I think that it is common to do it like that in rs232 communication.
Common doesn't yet mean correct. Java programmers use a lot of multithreading, so it's common, although in most cases unnecessary when brought to different grounds.


I worried about that. I think swapping would solve this?
Swapping in what way?


The threads must share just one thing, tha object of the foreign serial library, because this includes all stuff to access com-port.
Btw, i use this
http://www.codeproject.com/KB/system/serial.aspx

Have you tried QExtSerialPort?

donglebob
28th October 2008, 10:47
hi,


If you use non-blocking IO, you don't have to have any separate threads.
When I use there functions like "WaitForMultipleObjects", where the reader thread must wait for some events like "data arrived" or sth. like that, it will block the thread until data is available.
Meanwhile my writer thread could do some IO operations. Thats the reason why I need two threads.



Swapping in what way?Now my threads are children of the the serial-class and they use the pointer of parent to execute parent-functions.
When I switch parent to children and children to parent, every access to serial-class-object would run in the threads context and not in the main-app.



Have you tried QExtSerialPort?Yes, but it just allows blocking IO. overlapped IO is not implemented yet.

wysota
28th October 2008, 18:25
When I use there functions like "WaitForMultipleObjects", where the reader thread must wait for some events like "data arrived" or sth. like that, it will block the thread until data is available.
Meanwhile my writer thread could do some IO operations. Thats the reason why I need two threads.
Ok, but why block at all?


Yes, but it just allows blocking IO. overlapped IO is not implemented yet.

Not really. You can check bytesAvailable() and do read() only when there is something to read. In that case it returns immediately. Actually I think it will return immediately regardless if there is anything to read (I'm not sure of that though).

donglebob
28th October 2008, 23:15
Ok, but why block at all?

Uhmm, thats how WaitForMultipleObject works ?!?!
In the samples of MS, which I read, it was always blocked like this.
I have to wait for events...and if i understood it right the thread must wait somwhere and proceed after the wanted event arrived, to read the data etc... (data can be read immediately or with pending = while pending WaitFor......)

http://msdn.microsoft.com/en-us/library/ms810467.aspx#serial_topic4


Not really. You can check bytesAvailable() and do read() only when there is something to read. In that case it returns immediately. Actually I think it will return immediately regardless if there is anything to read (I'm not sure of that though).

As far as I know access to com at "same time" is not allowed in Qextserialport.
Checked the source again.
In Win32 it open com-port always with the last parameter NULL, that means
non-overlapped (synchrone) access.

wysota
29th October 2008, 00:19
Uhmm, thats how WaitForMultipleObject works ?!?!
Ok, but what is the practical reason of doing that? :) You can use asynchronous communication instead - no blocking, no problems.



As far as I know access to com at "same time" is not allowed in Qextserialport.
Checked the source again.
In Win32 it open com-port always with the last parameter NULL, that means
non-overlapped (synchrone) access.

But you only read if there is anything to read thus the call returns at once effectively making it asynchronous (although in theory it is still synchronous). I assure you you can read and write to the port "at the same time". In practice it is not "the same time" as you're using a single thread so you can either read or write because of a single flow, but you can do one immediately after the other. I assure you this works just fine in many-many applications out there.

Qt does most of it's I/O in buffered mode making it possible for your application to be even more "asynchronous". I don't exactly know how QExtSerialPort is implemented (I'm not very impressed with its quality) but it is known to work so I don't see a reason not to use it. The only thing that sucks with it is the necessity of using timers to poll the port. I've been meaning to fix this myself but currently I don't have time :(

donglebob
29th October 2008, 10:25
hi,

You know I want to use that overlapped IO....and have written the whole sing for that :)
It would make me cry to throw all of it away :D


Ok, but what is the practical reason of doing that? http://www.qtcentre.org/forum/../images/smilies/smile.png You can use asynchronous communication instead - no blocking, no problems.

I think i am using asynchronous comm. with a little break :-)
Ok maybe I misunderstood sth. How should I inform my thread that there is data? (without using WaitforMultipleObjects)

I have to react when an event arrives, so I can read from the port.
Is there another way without WaitFor.......?

wysota
29th October 2008, 13:07
Use a timer and check bytesAvailable() periodically. It's a bit dirty solution but tends to be acceptable in this case. If you want to synchronize threads, use the API Qt provides instead of the native one. In many cases it will be much faster. WaitForMultipleObjects() is an equivalent of a semaphore. Use QWaitCondition or similar instead.

donglebob
29th October 2008, 23:23
timer is a real dirty sollution. and doing like this sounds like polling with small breaks. thats what i want to/should avoid. I want to do that event-driven.

I think i will keep using WaitFor..... and the two threads...
but i must think on how two give both threads the ability to use the "foreign serial lib instance" to access the port.

Have to sleep one night..and maybe I will get an idea while sleeping :D

wysota
29th October 2008, 23:49
timer is a real dirty sollution. and doing like this sounds like polling with small breaks. thats what i want to/should avoid. I want to do that event-driven.

If you expect data to be available very often then this is fine as most of the time the poll will end up positive. If you expect data to be rare, then increase the timeout. Or use an adaptive timeout...