What Is The Right Threading Strategy For This? Topic is solved

[Xangis]

I have an application that is basically a glorified telnet client. There is a wxThread that processes incoming messages and adds them to a wxRichTextBox.

The UI can also cause that wxRichTextBox to be updated, mainly when the user issues commands that are echoed to that text box.

The trouble with this is that the wxRichTextBox is being accessed from more than one thread.

I've added a wxMutex that I lock every time I want to change info in the text box from the wxThread. I also overrode the paint event of the text box and lock the wxMutex inside that before calling the base's paint function and then unlocking.

That almost works -- until the text box gets to the point where it has so much text that it automatically adds a vertical scrollbar and I get a deadlock inside the OnPaint handler because adding the scrollbars triggers a paint event. It ain't pretty.

So, I could create a derived class where I override everything and lock everywhere and get a little ridiculous with things, but that doesn't seem like the right way to go about it.

Is there I way I can post the wxRichTextBox commands as events to the UI thread so it can process them in its own time? All I'm really doing is appending or clearing the text and changing fonts/colors.

I've been using WPF a lot lately, so I've gotten kind of used to just tossing a GUI event into the GUI message queue for it to handle, bypassing any threading issues. Is that way feasible with wxWidgets?

[spartygw]

I've been using WPF a lot lately, so I've gotten kind of used to just tossing a GUI event into the GUI message queue for it to handle, bypassing any threading issues. Is that way feasible with wxWidgets?

Bingo, that's exactly what you want to do. In the non-gui threads create an event and post it to the event handler.

See: http://wiki.wxwidgets.org/Custom_Events

-gw

[Xangis]

Thank you, that has been quite helpful.

I'm having a bit of trouble with posting events, though. I've scoured the historical posts, but haven't found a clear solution --

When I call wxEvtHandler::ProcessEvent everything works. When I call wxEvtHandler::AddPendingEvent, the event disappears into the aether and is never processed. I'm sure I've missed an ID or a connection, but can't figure out what's off.

This is done using "method 4" in the wiki link that was posted.

wxTextOutputEvent.h:

Code: Select all

#pragma once

#include "wx/wx.h"

extern const wxEventType wxTextWindowOutputEvent;

class wxTextOutputEvent: public wxCommandEvent
{
public:
	wxTextOutputEvent( wxString text, bool newline, wxColour foregroundColour,  wxEventType commandType = wxTextWindowOutputEvent, int id = 0 );
	wxTextOutputEvent( const wxTextOutputEvent &event );
	wxEvent* Clone() const { return new wxTextOutputEvent(*this); }
	wxString Text;
	bool Newline;
	wxColour ForegroundColour;
};

typedef void (wxEvtHandler::*wxTextOutputEventFunction)(wxTextOutputEvent &);

// This #define simplifies the one below, and makes the syntax less
// ugly if you want to use Connect() instead of an event table.
#define wxTextOutputEventHandler(func)                                         \
	(wxObjectEventFunction)(wxEventFunction)(wxCommandEventFunction)\
	wxStaticCastEvent(wxTextOutputEventFunction, &func)                    
 
// Define the event table entry. Yes, it really *does* end in a comma.
#define EVT_TEXT_OUTPUT(id, fn)                                            \
	DECLARE_EVENT_TABLE_ENTRY( wxTextWindowOutputEvent, id, wxID_ANY,  \
	(wxObjectEventFunction)(wxEventFunction)                     \
	(wxCommandEventFunction) wxStaticCastEvent(                  \
	wxTextOutputEventFunction, &fn ), (wxObject*) NULL ),

wxTextOutputEvent.cpp:

Code: Select all

#include "wxTextOutputEvent.h"

const wxEventType wxTextWindowOutputEvent = wxNewEventType();

wxTextOutputEvent::wxTextOutputEvent( wxString text, bool newline, wxColour foregroundColour, wxEventType commandType, int id )
: wxCommandEvent(commandType, id)
{
	Text = text;
	Newline = newline;
	ForegroundColour = foregroundColour;
}

wxTextOutputEvent::wxTextOutputEvent(const wxTextOutputEvent &event)
{
	Text = event.Text;
	Newline = event.Newline;
	ForegroundColour = event.ForegroundColour;
}

I've tried using the event table:

Code: Select all

BEGIN_EVENT_TABLE(wxBasternaeDlg, wxFrame)
        ...
	EVT_TEXT_OUTPUT(wxID_ANY, wxBasternaeDlg::FlushText )
	...
END_EVENT_TABLE()

I've also tried connecting the event directly in the constructor:

Code: Select all

this->Connect(wxID_ANY, wxTextWindowOutputEvent, wxTextOutputEventHandler( wxBasternaeDlg::FlushText), NULL, this);

Does anything look off to you or is there anything I should check?

My calls look like this:

Code: Select all

wxEvtHandler::AddPendingEvent(wxTextOutputEvent(inputbuf, true, clrWhite ));

[spartygw]

Hi again.

The call to wxEvtHandler::AddPendingEvent looks curious to me. I don't think that's a static class function of wxEvtHandler so I'm not sure how that is compiling for you.

Try this instead:

Code: Select all

wxTheApp->GetTopWindow()->GetEventHandler()->AddPendingEvent(myEvent);

or

Code: Select all

wxPostEvent(wxTheApp->GetTopWindow()->GetEventHandler(), myEvent);

Let me know how it goes.
-gw

[DavidHart]

Hi,

I didn't look in detail, but the call should be something like:
pBasternaeDlg->GetEventHandler()->AddPendingEvent(wxTextOutputEvent(inputbuf, true, clrWhite ));
where pBasternaeDlg is a pointer to an instance of the class with the event table (or the Connect() statement).

Regards,

David

[Debster]

Hi,

I also have a pool of worker threads, for doing some communication stuff. So I'm using this:

wxConnectionEvent event(wxConnectionEvent::disconnect);
::wxPostEvent(wxFssApp::getFrame().GetEventHandler(), event);

wxConnectionEvent is a user defined event. The Mainframe handles all the gui stuff.

Hope that helps.
Regards
Debster

[Xangis]

It feels like I'm so close, but just not quite there yet. Here are all of the variations I've tried and none have worked:

Code: Select all

this->AddPendingEvent(wxTextOutputEvent(inputbuf, true, clrWhite ));
wxEvtHandler::AddPendingEvent(wxTextOutputEvent(inputbuf, true, clrWhite ));
this->GetEventHandler()->AddPendingEvent(wxTextOutputEvent(inputbuf, true, clrWhite ));
wxPostEvent(this->GetEventHandler(),wxTextOutputEvent(inputbuf, true, clrWhite ));
_txtOutput->GetEventHandler()->AddPendingEvent(wxTextOutputEvent(inputbuf, true, clrWhite ));

I have to wonder if there's a problem with using wxID_ANY, the NULL in the connect, or with not passing the wxEventType in the call to the wxTextOutputEvent constructor, but I'm really not sure whether any of those could be a factor.

The application I'm working on has a wxFrame with a child wxPanel and the control I'm trying to write to (_txtOutput) is a wxRichTextCtrl attached to the wxPanel. I can't imagine there's anything special about that layout, but who knows?

[spartygw]

If you want to attach a collection of cpp and header files I'll build your app and see if I can find the problem.

When I use custom events I do it a little differently, I use method 1 in the link I posted.

Don't get too discouraged, I'm sure you'll get it.

-gw

[doublemax]

Where does the secondary thread come into play? All the code lines you showed can not have been made from a wxThread context.

Please show the surrounding code where you create and send the event.

Also:

Code: Select all

wxTextOutputEvent( wxString text, bool newline, wxColour foregroundColour,  wxEventType commandType=wxTextWindowOutputEvent, int id=0 ;

Shouldn't the default value for id be wxID_ANY ?

Additionally, if you send events from secondary threads, the Clone() method must create deep copies of wxString objects:
http://forums.wxwidgets.org/viewtopic.p ... 503#103503

[Xangis]

Thank you for the tip on the string copy. I stepped through the copy code and verified that I do get at least as far as the event being copied.

The output wxRichTextCtrl is updated from two places. The sample event I posted is called from the OnKey handler for the input window (I echo all commands to the output window before sending across the network).

The other place the event will be posted from is the network thread:

Code: Select all

void * wxBasternaeDlg::Entry()
{
	while( !this->TestDestroy())
	{
		ProcessNetwork();

		// throttle to keep from flooding the event queue
		wxMilliSleep(20);
	}
	return 0;
}

The 'process network' function reads messages from the network thread, does some processing and character substitution (probably too much irrelevant code to post) and makes this call when adding test to the window:

Code: Select all

wxEvtHandler::AddPendingEvent(wxTextOutputEvent(output, true, *_foregroundColor ));

(and of course, some of the other variations I posted above)

As I understand it, the event should behave the same when posted from the window thread or from the network thread, and it shouldn't matter from which thread I post them to the event queue if I have them working right. If I could just get the thing to process once from either thread I'll be golden.

I don't suppose there's some event processing code I could step through somewhere in the internals of wxWidgets?

[Xangis]

Well, it looks like I've come up with a workaround. It doesn't solve the problem of coming up with a new event type that works, but it does the trick using a command event.

The event table has:

Code: Select all

	EVT_COMMAND(IDM_TEXT_UPDATE, wxEVT_NULL, MyDlg::FlushText )

Instead of embedding parameters in my event, I drop them into a simple StringEntry class and store them locally for later use in a mutex-enclosed variable:

Code: Select all

void MyDlg::AddStringEvent(wxString text, bool newline, wxColour color )
{
	StringEntry entry = StringEntry(text, newline, color);
	_pendingStringOutputMutex->Lock();
	_pendingStringOutputs->push_back(entry);
	_pendingStringOutputMutex->Unlock();
	_txtOutput->AddPendingEvent(wxCommandEvent(wxEVT_NULL, IDM_TEXT_UPDATE));
}

And, in the processing code:

Code: Select all

void MyDlg::FlushText(wxCommandEvent&)
{
	_pendingStringOutputMutex->Lock();
	StringEntry entry = _pendingStringOutputs->front();
	_pendingStringOutputs->pop_front();
	_pendingStringOutputMutex->Unlock();
	RemoveUnprintableASCIIChars(&entry.Text);
	_txtOutput->BeginTextColour(entry.ForegroundColour);
	if( entry.Newline )
	{
		_txtOutput->AppendText(entry.Text);
		_txtOutput->AppendText(_("\n"));

It feels a bit silly, but it does do the trick with a minimum of fuss since I can rely on events that already exist. I guess sorting out the guts of custom events is an adventure I can postpone for another day. Thank you all for your contributions.

[spartygw]

Hey dude, I just can't let it go.

Here's a test app I wrote this morning to demonstrate a thread being spun off from the main GUI and posting a custom events.

Take a look, hopefully it's readable

Code: Select all

#include <wx/wx.h>

//-----------------------
// main application class
class MyApp : public wxApp
{
public:
      virtual bool OnInit();
};


//---------------------------------------
// this would usually go in a header file
BEGIN_DECLARE_EVENT_TYPES()
  extern const wxEventType wxEVT_STATUS_UPDATE;
END_DECLARE_EVENT_TYPES()

// ------------------------------------------
// define data classes that will be passed in 
// wxCommandEvents as void *
class wxStatusUpdateData
{
  public:
    wxString msg;
};

class MyThread : public wxThread
{
    private:
	    virtual void *Entry()
      {
        wxCommandEvent myEvent(wxEVT_STATUS_UPDATE);
        wxStatusUpdateData *data = new wxStatusUpdateData();
        data->msg = "foo";

        myEvent.SetClientData(static_cast<void *>(data));

        for (int i=0; i<10; i++)
        {
            data->msg.Printf("Update %d", i);
            wxPostEvent(wxTheApp->GetTopWindow()->GetEventHandler(), myEvent);
            wxSleep(2);
        }

        return 0;
      }
};


// ---------------
// the main window
class MyFrame : public wxFrame
{
public:
        MyFrame(wxWindow *);
        wxStatusBar *m_statusBar;

        DECLARE_EVENT_TABLE()
        void OnStatusUpdate(wxCommandEvent &);

        MyThread *m_thread;
};


//-------------------------------------------------------
// everything before this point would be header material,
// and this would go in the cpp file
DEFINE_EVENT_TYPE(wxEVT_STATUS_UPDATE);

BEGIN_EVENT_TABLE(MyFrame, wxFrame)
  EVT_COMMAND(wxID_ANY, wxEVT_STATUS_UPDATE, MyFrame::OnStatusUpdate)
END_EVENT_TABLE()

// -------------------------------------------------
// just create a simple window with a status bar
// that will be updated by the thread posting events
MyFrame::MyFrame(wxWindow * parent)
: wxFrame(parent, -1, "Test", wxPoint(100,100), wxSize(400,100))
{              
    m_statusBar = CreateStatusBar( 1, wxST_SIZEGRIP, wxID_ANY );
    m_statusBar->SetStatusText("Waiting for first update.");
    Centre();

    // spin off the thread
    m_thread = new MyThread();
    m_thread->Create();
    m_thread->Run();
}

// -----------------
// wx event callback
void MyFrame::OnStatusUpdate(wxCommandEvent &evt)
{ 
  wxStatusUpdateData *data = static_cast<wxStatusUpdateData *>(evt.GetClientData());


  if (data)
  {
    m_statusBar->SetStatusText(data->msg);
  }
}

IMPLEMENT_APP(MyApp)

bool MyApp::OnInit()
{
      MyFrame * frame = new MyFrame(NULL);   
      frame->Show(); 
      SetTopWindow(frame);
      return true;
}

-gw