Ice::Thread and GUI with Qt

[catalin]

Hi!

I have an Ice application, where the server makes callbacks to the client, by means of proxy pointers which the client passes to it.

The client is to update its GUI based on the response received from the server, so I figured out that the right place to initialize the GUI is in the constructor of the callback proxy, which does something like the following, where the code in blue constructs a new Ice::Thread:

// -----------------------------------------------------------------------------------
ImageViewerI::ImageViewerI(int argcc, char *argvv[]) {
// first initialize the imageViewer to some defaults
  argc = argcc;
  *argv = argvv[0];
  
  app = new QApplication(argc, argv);
  ...  
  imageViewer = new ImageViewer(params.imageWidth, params.imageHeight, 32);
  
  app->setMainWidget(imageViewer);
  imageViewer->show();
  
  // let another thread run the GUI loop
  guiThread = new qApplicationThread(argc, argv, app) ;
  threads.push_back(guiThread->start());
  ...
}

What I am trying to do is not to block the thread which receives the callback, with the GUI event loop. As you can see from the implementation of the qApplicationThread class, its sole purpose is to call guiApp->exec(); which blocks the thread in which it is called:

qApplicationThread::qApplicationThread(int argc, char *argv[],
				       QApplication *qa) {
  guiApp = qa;
}

void 
qApplicationThread::run() {
  guiApp->exec();
}

The servant makes the callback by calling the following method on the proxy object it has received. When the call is made, I update the image in the GUI:

// -----------------------------------------------------------------------------------
void
ImageViewerI::acceptPacket(const ::dacapoCB::DCPacket& dcp, const ::Ice::Current&) {
  ...
  char *image = (char *)(&dcp[0]);
  imageViewer->updateImage(image + _DACAPO_PAYLOAD_OFFSET_, imageSize);  
  ...
  }
}

This application design seems to be somehow faulty, because the client often dies out at startup with a segmentation fault, caused by a call deep down in the Qt library. When it does not die at start up, it always receives a Xlib: unexpected async reply (sequence 0x180)! error, after running a while. Sometimes is receives 2300 images before it causes this error.

I think the issue is not related so much to Ice, but is a basic design issue of multithreaded GUI applications.

Any good advice?

Thanks,
Catalin

[xdm]

Hi cataling

the problem of your code is that qt don't suport that you call gui operations from other thread that is not the qt main thread where Qt::eventLoop is runing,

the correct way to solve this proble is dispacth a custom event from the oder thread and intercept this even't in your widget class

to dispach a custom event yo main call static function QApplication::postEvent( QObject * receiver, QEvent * event ) Note: This function is thread-safe when Qt is built withthread support.

Example

// A custom event class

class ProgressEvent : public QCustomEvent
{
	private:
	int total;
	int part;
	public:
	
	ProgressEvent(int total,int part);
	int getTotal();
	int getPart();
};


// A QObject class to install as eventFilter in widgets that's need intercept your customeEvent
//this EventFilter is instaled in a QWidget calling void installEventFilter ( const QObject * filterObj ) a passing this object as filterObject param

class ProgressEventFilter : public QObject
{
	Q_OBJECT
	public:
	ProgressEventFilter(QObject* parent=0,const char* name=0);
	protected:
        bool eventFilter(QObject *o,QEvent *e);
};

// A Qthread class to invoque Ice methods from a diferent thread

class FileLoader : public QThread
{
	private:
	FilePrx target;
	string path;
	string action;
	QObject* listener;
	public:
	FileLoader();
	FileLoader(const FilePrx& target,const string& path,const string& action,QObject* listener=0);
	
	bool Upload(const string& source);
	bool Download(const string& target);
	bool toString();
	
	//QTrhead virtuals abstract
	
	virtual void run();
};

FileLoader implementation

FileLoader::FileLoader()
{

}

FileLoader::FileLoader(const FilePrx& target,const string& path,const string& action,QObject* listener)
{
	this->target=target;
	this->path=path;
	this->action=action;
	this->listener=listener;
}

bool FileLoader::Upload(const string& filePath)
{
	bool retval=false;
	if(QFile::exists(filePath))
	{
		ifstream source(filePath.c_str(),ios::binary);
		if(source)
		{
			/*Calculate the file size*/
			long fileSize;
			source.seekg(0,ios::end);
			fileSize=source.tellg();
			source.seekg(0,ios::beg);
		
			cout <<"File size is: "<<fileSize<<endl;
			
			int chunkSize=target->getChunkSize();
			cout<<"chunkSize is: "<<chunkSize<<endl;
			
			int totalChunks = fileSize/chunkSize;
			cout<<"totalChunks: "<<totalChunks<<endl;
			long chunk_pos=0;
			while(!source.eof())
			{
				int currentProgress=source.tellg();
				if(currentProgress!=-1)
				{
                                         //Creating Out custom Event
					ProgressEvent* progress=new ProgressEvent(fileSize,currentProgress);
					cout<<source.tellg()<<"/"<<fileSize<<endl;
					//Thispathc the event to QApplication
QApplication::postEvent(listener,progress);
				}
				/*Creating a new chunk for add in to the file*/
				ChunkPtr chunk=new ChunkI();
				chunk->pos=chunk_pos;
				chunk_pos++;
				
				chunk->content.resize(chunkSize);
				source.read(reinterpret_cast<char*>(&chunk->content[0]),chunk->content.size());
				if((source.eof())&&(source.gcount()<chunkSize))
					chunk->content.resize(source.gcount());
				/*
					upload the new chunk to target File
				*/
				target->addChunk(chunk);
				
				msleep(20);
				//emit progress(source.tellg(),fileSize);
			}
			ProgressEvent* progress=new ProgressEvent(fileSize,fileSize);
			QApplication::postEvent(listener,progress);
			cout<<"source file upload ok"<<endl;
		}
		source.close();
		retval=true;
	}
	else
		cout<<"File: "<<filePath<<" no Exist"<<endl;
	return retval;
}


bool FileLoader::Download(const string& filePath)
{
	cout<<"go to download file to target : "<< filePath<<endl;
	bool retval=false;
	QFile targetFile(filePath);
	if(targetFile.open(IO_WriteOnly))
	{
		long chunks=target->countChunks();
		long chunkSize=target->getChunkSize()*sizeof(Ice::Byte);
		for(int cont=0;cont<chunks;cont++)
		{
			ChunkPrx chunk=target->readChunk(cont);
			Bytes content=chunk->read();
			targetFile.writeBlock(reinterpret_cast<char*>(&content[0]),content.size());	
		}
		targetFile.close();
		FileDownloaded* downloaded=new FileDownloaded(filePath);
		QApplication::postEvent(listener,downloaded);
		cout<<"File Downloaded OK"<<endl;
		retval=true;	
	}
	else
		cout<<"error creating file: "<<filePath<<endl;
	return retval;
}

bool FileLoader::toString()
{
	QString str;
	QTextStream ts( &str, IO_WriteOnly );
	long chunks=target->countChunks();
	for(int cont=0;cont<chunks;cont++)
	{
		ChunkPrx chunk=target->readChunk(cont);
		Bytes content=chunk->read();
		Bytes::const_iterator it_1;
		for(it_1=content.begin();it_1!=content.end();it_1++)
		{
			char* slot=(char*)malloc(sizeof(Ice::Byte));
			memcpy((void*)slot,(void*)(&(*it_1)),sizeof(Ice::Byte));
			std::cout<<slot;
			
			delete(slot);
		}
		std::cout<<endl;
	}
	return true;
}


void FileLoader::run()
{
	if(target)
	{
		cout<<"FileLoader run action: "<<action<<" path: "<<path<<endl;
		if(action=="download")
			Download(path);
		else if(action=="upload")
			Upload(path);
		else if(action=="toString")
			toString();
	}
}

A widget thas's install the customEventFilter

#include <FileUploaderView.h>


FileUploaderView::FileUploaderView(QWidget* parent,const char* name,string fileName)
	:QWidget(parent,name)
{
	qDebug("FileUploaderView constructor");
	lblFileName=new QLabel(this,"lblFileName");
	
	QString lblText=QString("Uploading %1").arg(fileName.c_str());
	lblFileName->setText(lblText);
	
	//Creating a custom event filter
ProgressEventFilter* eventFilter=new ProgressEventFilter(this);	
	progressBar=new QProgressBar(this,"progressBar");
      //Instaling the event filter
	progressBar->installEventFilter(eventFilter);	
	
	QVBoxLayout* mainLayout=new QVBoxLayout(this,2,2,"mainLayout");
	mainLayout->addWidget(lblFileName);
	mainLayout->addWidget(progressBar);
	qDebug("FileUploaderView constructor");
}


QProgressBar* FileUploaderView::getProgressBar()
{
	return progressBar;
}

the event and event filter implementation

#include <ProgressEvent.h>

ProgressEventFilter::ProgressEventFilter(QObject* parent,const char* name)
	:QObject(parent,name)
{

}

bool ProgressEventFilter::eventFilter(QObject *o,QEvent *e )
{
	if ( e->type() == 10001 ) 
	{
		// special processing for progress event
		ProgressEvent* p =(ProgressEvent*)e;
		qDebug( "Porgress %d of %d", p->getTotal(),p->getPart());
		QProgressBar* bar=(QProgressBar*)o;
		bar-> setProgress (p->getPart(),p->getTotal());
		return TRUE; // eat event
	} 
	else 
	{
            // standard event processing
            return FALSE;
        }
}

ProgressEvent::ProgressEvent(int total,int part)
	:QCustomEvent(10001)
{
	this->total=total;
	this->part=part;
}
int ProgressEvent::getTotal()
{
	return total;
}


int ProgressEvent::getPart()
{
	return part;
}

I hope this code help you

[catalin]

Thanks, I will look at events and custom events now, and try to see how they can help.

One thing which worries me a little is the fact that my images arrive in a thread in a memory location pointed to by a char *. But this data is to be painted on screen by another thread, which gets this char *. In a way the two threads share data (memory) across thread boundaries. There are no concurrent accesse issues here, but still it seems so artificial to share data (memory) in this way. Is this fundamentally wrong?

Thanks,
Catalin

[matthew]

Originally posted by catalin
Thanks, I will look at events and custom events now, and try to see how they can help.

One thing which worries me a little is the fact that my images arrive in a thread in a memory location pointed to by a char *. But this data is to be painted on screen by another thread, which gets this char *. In a way the two threads share data (memory) across thread boundaries. There are no concurrent accesse issues here, but still it seems so artificial to share data (memory) in this way. Is this fundamentally wrong?

Thanks,
Catalin

xdm is exactly correct. You cannot call Qt UI methods from any thread other than main. There is a chapter in the Qt documentation on exactly this topic. I recommend that you read it.

With respect to your concern. You should not pass the raw bytes to your main thread, but rather pass dcp in the QCustomEvent that you implement. What I recommend is that you setup a servant that has a copy of the display widget implementation. Perhaps something like this:

class ImageAcceptPacketEvent : public QCustomEvent
{
public:

   ImageAcceptPacketEvent(const ::dacapoCB:: DCPacket& data) :
     dcp(data)
  {
  }

   const ::dacapoCB:: DCPacket dcp;
};

Then in the servant implementation you create an instance of this event and post it to the widget.

void
ImageViewerI::acceptPacket(const ::dacapoCB:: DCPacket& dcp, const ::Ice::Current&)
{
  qApp->postEvent(_displayWidget, new ImageAcceptPacketEvent(dcp));
}

And then the widget implements customEvent as follows:

class ImageViewerWidget : public QWidget
{
public:

   virtual void customEvent(QCustomEvent*); // Called by the Qt runtime.
...
};

void
ImageViewerWidget::customEvent(QCustomEvent* ce)
{
   ImageAcceptPacketEvent* e = dynamic_cast<ImageAcceptPacket*>(ce);
   if(e != 0)
  {
     char *image = (char *)(&e->dcp[0]);
    updateImage(image + _DACAPO_PAYLOAD_OFFSET_, imageSize);  
  }
}

Of course, if there are lots of events then you can do something more elegant in customEvent.

Regards, Matthew

[catalin]

Thanks matthew (and everyone else) especially as this is obviously more Qt related then Ice-related. You are very kind

I have read the Qt doc about the main thread and multithreading when I first encountered the Xlib problem, but I was not aware that custom events would be an alternative to what I was trying to do, until xdm mentioned it for me. I will try it out as soon as I finish programming some other modules.

I am operating at 30 images per sec, on the maximum, so I am very curios if postevent will be processed often enough by Qt's runtime. I would prefer to be able to use sendevent, which I understand is synchronous, and publishes the event when it is called instead of placing it in a queue, but I could not see any mention that it is threadsafe, as postevent is documented to be.

So I am living in excitement....

Looking aside from the Qt issue, I still wonder if it is fundamentally correct to share memory between threads by means of pointers.

Some other place I am doing the following:

queue<element *> buffer; // FIFO
IceUtil::Mutex mutex;

// new thread objects which receive pointers to both the buffer and the mutex
IceUtil::ThreadPtr tp1 = new A(&buffer, &mutex);
IceUtil::ThreadPtr tp2 = new B(&buffer, &mutex);

    // both tp1 and tp2 place elements in the buffer (queue) like this:
    mutex->lock();
    buffer->push(e);
    mutex->unlock();

    // both tp1 and tp2 read the first element from the queue like this:
    mutex->lock();
    element *e = 0;
    if ( buffer->size() > 0 ) {
      e = buffer->front();
      buffer->pop();
    }
    mutex->unlock();

Is this all done by the book?

Again, thank you!
Catalin

[matthew]

Originally posted by catalin
Thanks matthew (and everyone else) especially as this is obviously more Qt related then Ice-related. You are very kind

I have read the Qt doc about the main thread and multithreading when I first encountered the Xlib problem, but I was not aware that custom events would be an alternative to what I was trying to do, until xdm mentioned it for me. I will try it out as soon as I finish programming some other modules.

I am operating at 30 images per sec, on the maximum, so I am very curios if postevent will be processed often enough by Qt's runtime. I would prefer to be able to use sendevent, which I understand is synchronous, and publishes the event when it is called instead of placing it in a queue, but I could not see any mention that it is threadsafe, as postevent is documented to be.

This will not work, you must use post event to transfer the image to the main thread to be processed by the UI. The event queue will be processed at whatever speed your computer is capable of. If the software cannot keep up, then you will have to throttle the processing of the images, by dropping some.

So I am living in excitement....

Looking aside from the Qt issue, I still wonder if it is fundamentally correct to share memory between threads by means of pointers.

This is a more general programming question. It is possible to share memory however you want, as long as it is suitably protected.

Some other place I am doing the following:

queue<element *> buffer; // FIFO
IceUtil::Mutex mutex;

// new thread objects which receive pointers to both the buffer and the mutex
IceUtil::ThreadPtr tp1 = new A(&buffer, &mutex);
IceUtil::ThreadPtr tp2 = new B(&buffer, &mutex);

    // both tp1 and tp2 place elements in the buffer (queue) like this:
    mutex->lock();
    buffer->push(e);
    mutex->unlock();

    // both tp1 and tp2 read the first element from the queue like this:
    mutex->lock();
    element *e = 0;
    if ( buffer->size() > 0 ) {
      e = buffer->front();
      buffer->pop();
    }
    mutex->unlock();

Is this all done by the book?

Again, thank you!
Catalin

It is pretty inefficient. I would look at using a condition variable, or an Ice monitor. You can look at any MT programming book, or the Ice manual (check, for example, ice/demo/IceUtil/workqueue) for lots of examples of how to efficiently write producer/consumer style applications.

Regards, Matthew