events, secondary threads & app remote control

[bsenftner]

Let's say one has a multi-threaded wxWidgets app, with multiple windows, dialogs, toolbars with icon/menu controls, panels and so forth. These are constructed in the conventional manner with wxCommandEvents as the event type used to pass requests around the application. All GUI operations occur in the wxWidgets thread, and because the application is multi-threaded, operations that occur in other threads are delivered to the GUI thread via events. The various windows each have a background thread for time consuming operations (exporting) as well as each window receives data from a separate thread via a callback (delivering video frames, actually).

If I am understanding the information here viewtopic.php?f=1&t=45924&p=191466&hili ... nt#p191466 then the above described architecture needs to be using wxThreadEvents because strings in the events are being wxQueueEvent()'ed around by the multiple threads.

Now, to complicate the requirements a bit, let's say the same wxWidgets application has a REST server running on one of these other threads, and among the different types of data that can be received from the REST thread is remote control of the GUI. These remote controls take the form of operations such as "open window with guid(g)", "play video in window with guid(g)", "close window with guid(g)". Where the "guid(g)" portion is a string to identify what is being operated upon. So, for REST communications to the GUI thread to be safe, they need to use wxThreadEvents anywhere a guid-string needs to be wxQueueEvent()'ed for identification of what to modify.

Plus, the GUI elements that are constructed receiving wxCommandEvents (close box, menu/icon handlers, etc) need to have an intermediary routine that catches wxThreadEvents (from other threads) and converts those into wxCommandEvents if those requests are things like triggering the onClose handler, or a menu/icon handler. Is that correct? One cannot pass a wxThreadEvent to a routine expecting a wxCommandEvent, unless there is some handy type converter?

I am currently working to fix the above described architecture. However, it is implemented with wxCommandEvents rather than wxThreadEvents, and only seems to work - the background threads sometimes crash, and the GUI experiences unpredictable crashes. I suspect it may be the guid strings in the wCommandEvents - they need to be in wxThreadEvents. Right?

[alys666]

do not panic on strings given to events.
you can clone given string manually, giving not actual string to event(if you are afraid - it won't be cloned) but clone it in place.
not Event(...my_string...), but Event (...my_string.Clone()). also you must clone somehow any data, which could be shared between threads via events, but is not given by copy.
thread safety idea is easy - or give full copy of data to another thread, or give by reference, but don't touch it, or share the same data, touch it, but protect by mutexes or critical sections.
in your architecture better to use wxThreadEvent to communicate between threads(or some custom class of event, if you need), where main frame gets this events from his build-in queue(for wxThreadEvents addressed to him), decodes, and calls some actions on gui).. it's how my system works.

[bsenftner]

Thank you for this information. So in places where data passes between threads, a wxThreadEvent should be used. Most important, strings should be cloned.

In the situation where one creates an event to send, either a wxCommandEvent or wxThreadEvent, if the event is going into wxQueueEvent() then the event is created with a "new" and not as a local variable, correct? wxQueueEvent() must delete the event pointer passed to it, right?

[bsenftner]

Hmmm... something is not right. I tried a small test case where I converted one event used by communication between threads from using a wxCommandEvent to a wxThreadEvent. However, the event handler is not called when a wxThreadEvent, but is called when a wxCommandEvent.

In my application I have a sqlite database that is modified in a secondary thread. When updates are complete the application receives a callback within the secondary thread's context. In the callback I generate an event to update the GUI, delivering the event with wxQueueEvent. The event handler is then supposed to generate the new display from reading the sqlite database.

In this small test case, a wxThreadEvent is not really required, because no data such as a string is being passed. However, the wxThreadEvent should be received by the handler, but is not. In the code below I show the wxThreadEvent implementation bits, with the necessary changes to be a wxCommandEvent version commented in the nearby lines. See any reasons the wxThreadEvent does not work?

in a header:

Code: Select all

///////////////////////////////////////////////////////////////////////////////////
// define an event to be used outside the GUI thread to trigger Gallery refresh
wxDECLARE_EVENT(wxEVT_GalleryInitEvent, wxThreadEvent); // wxCommandEvent);
///////////////////////////////////////////////////////////////////////////////////

In a cpp file:

Code: Select all

// wxDEFINE_EVENT(wxEVT_GalleryInitEvent, wxCommandEvent);
wxDEFINE_EVENT(wxEVT_GalleryInitEvent, wxThreadEvent);

wxBEGIN_EVENT_TABLE(CX_GalleryPanel, wxPanel)

EVT_THREAD(wxEVT_GalleryInitEvent, CX_GalleryPanel::OnInitialize)
// EVT_COMMAND(wxID_ANY, wxEVT_GalleryInitEvent, CX_GalleryPanel::OnInitialize)

wxEND_EVENT_TABLE()

///////////////////////////////////////////////////////////////////////////////
// used by secondary threads to trigger gallery refreshes
void CX_GalleryPanel::TriggerInitEvent(void)
{
	if (!this)
		return;

	// wxCommandEvent* evt = new wxCommandEvent(wxEVT_GalleryInitEvent);
	wxThreadEvent* evt = new wxThreadEvent(wxEVT_GalleryInitEvent);
	wxQueueEvent( this, evt );
}

////////////////////////////////////////////////////////////////////////////////
// void CX_GalleryPanel::OnInitialize(wxCommandEvent& event)
void CX_GalleryPanel::OnInitialize(wxThreadEvent& event)
{
	if (!mp_mainframe)
		return;

	// update gallery display
	(void)Initialize(mp_mainframe);
}

[alys666]

1. forget about static event tables - this style is old.
use Bind(...) - dynamically attach/detach event handler
2. if you want the frame, handling events from secondary thread, you must say somewhere(for example in constructor)
this->Bind(wxEVT_THREAD,&MyFrame::onAsyncThreadEvent,this);
here you bind method MyFrame::onAsyncThreadEvent to events of kind wxEVT_THREAD - class wxThreadEvent has this kind.
so if secondary thread posts wxThreadEvent to MyFrame instance, method - onAsyncThreadEvent of this insatnce will be called asynchronously.

minimal decoration

Code: Select all

class MyFrame:public wxWindow {

	MyFrame():wxWIndow(...){
		this->Bind(wxEVT_THREAD,&MyFrame::onAsyncThreadEvent,this);
	}
	
	void onAsyncEvent(wxThreadEvent &fe){... }
}

	//code to send message from secondary thread
	//variant 1 - i use
	wxThreadEvent *le = new wxThreadEvent(); //create on heap
	le->SetId(_uid); 
	le->SetInt(fi);	
	le->SetString(fval.Clone());  // here we clone only string given to event - faster
	_que->QueueEvent(le); 

	//variant 2
	wxThreadEvent le; //create on stack
	le->SetId(_uid); 
	le->SetInt(fi);	//which info we are sending to destination
	le->SetString(fval);  // here we do not clone the string
	_que->QueueEvent(le.Clone());  //but we clone the event entirely, le.Clone() will create on heap a full copy of stack created le

in your case, the thread, updating database, at the end of operation must call his function(better to make special function to
decrease code, if you want to send different messages)

Code: Select all

MyThread::sendMessage(int fmessage_id) {
	wxThreadEvent *le = new wxThreadEvent(); //create on heap
	le->SetId(DataBaseUpdateThreadGuid); //some unic guid given to this thread(because different secondary threads could post	to mainframe))
	le->SetInt(fmessage_id); //message id	
	//le->SetString(fval.Clone());  // if you do not need to attach string to event - just omit this line
	_dst->QueueEvent(le); //_dst here - the MainFrame instance, given to thread as parameter in constructor(as destination of his messages)
}

in my code i just wrote my base class, inherited from wxThread with function - sendMessage(...), and all my threads are based on it - and I can easy send messages to given to the thread destinations.

[doublemax]

a) there is nothing wrong with using static event tables
b) if you use the combination wxThreadEvent *and* QueueEvent(), there is no need to clone anything yourself. It will happen internally.

The most likely reason why you didn't receive the event is this:

Code: Select all

EVT_THREAD(wxEVT_GalleryInitEvent, CX_GalleryPanel::OnInitialize)

The first parameter is an ID, try wxID_ANY here.

One thing you need to remember when using wxThreadEvent is that it does not derive from wxCommandEvent. This means it does not propagate upwards in the window hierarchy, which can be inconvenient.

If you want to keep using wxCommandEvent, just use QueueEvent() and clone the string member like alys666 showed in his "variant 1".

[alys666]

i do not recommend to use wxCommandEvent to be fired from user threads(it's just my IMHO) - it's events used by wxWidgets to be fired by wxWidgets controls... so if secondary thread fires them - he is simulating reaction of controls, and it makes things more complex.
but wxThreadEvent is just a thing intended for inter-thread communication, not a controls reaction.

[bsenftner]

Doublemax,

The definition of EVT_Command is

Code: Select all

#define EVT_COMMAND(winid, event, func) \
    wx__DECLARE_EVT1(event, winid, wxCommandEventHandler(func))

While EVT_THREAD is:

Code: Select all

#define EVT_THREAD(id, func)  wx__DECLARE_EVT1(wxEVT_THREAD, id, wxThreadEventHandler(func))

So it looks like my use of EVT_THREAD as

Code: Select all

EVT_THREAD( wxEVT_GalleryInitEvent, CX_GalleryPanel::OnInitialize)

is correct. Am I missing something?

[doublemax]

Code: Select all

EVT_THREAD( wxEVT_GalleryInitEvent, CX_GalleryPanel::OnInitialize)

wxEVT_GalleryInitEvent is an event type. But the first position in the macro is an ID. This ID must match the ID in the event, otherwise the handler will not be called. But the id you set in the event is le->SetId(DataBaseUpdateThreadGuid);

Have you tried with wxID_ANY instead of wxEVT_GalleryInitEvent as i suggested?

[bsenftner]

I believe I am in a NIH (not invented here) catch-22. My CTO does not use any custom events in his wxWidgets projects; he simply uses wxEVT_TEXT for all customized event situations, and then uses Connect() with unique IDs to bind different function pointers to the unique IDs. He has success with other projects that heavily multi-threaded; He's never used a wxThreadEvent nor made a custom event (wxDeclareEvent()/wxDefineEvent()), and is telling me I'm not allowed, they don't work for him.

I can see that he method does work. As long as any text I set in the wxEVT_TEXT is cloned, it should be thread safe. Do you foresee this method causing problems?

[doublemax]

As long as any text I set in the wxEVT_TEXT is cloned, it should be thread safe. Do you foresee this method causing problems?

Impossible to say without seeing the "bigger picture". An wxEVT_TEXT event (like all other events) is handled in the main thread. It all depends on which data is shared across threads and how.