Author Topic: Help on wxWidgets multi threading management  (Read 18677 times)

Offline dje

  • Lives here!
  • ****
  • Posts: 683
Help on wxWidgets multi threading management
« on: April 04, 2007, 11:42:16 pm »
Hi all !

I am working on the ThreadSearch plugin and I have a problem with wxWidgets multi threading management.
Pecan, I know you advised me to avoid using graphical widgets in worker thread but I'd like to understand better that point of the wxWidgets framework.

My worker thread uses a cbStyledTextControl.
I found in wxWidgets documentation on wxMutexGuiEnter() :
Quote
This function must be called when any thread other than the main GUI thread wants to get access to the GUI library.

My worker thread uses the control in this way:
Code
wxMutexGuiEnter();
FindInFile(*control, m_FilePaths[i], searchFlags);
wxMutexGuiLeave();

During the search, I can edit text, use contextual menu, use my Message tab ThreadSearch panel scrollbars but as soon as I enter the event table with a button click for example, both threads freeze, the worker one on a wxMutexGuiEnter() call.
Event handler code:
Code
void ThreadSearchView::OnBtnSearchClick(wxCommandEvent &event)
{
// User clicked on Search/Cancel
if ( m_pFindThread != NULL )
{
StopThread();
}
else
{
// We start the thread search
cbFindReplaceData findData;
m_ThreadSearchPlugin.GetFindData(findData);
findData.findText = m_pCboSearchExpr->GetValue();
ThreadedSearch(findData);
}
    event.Skip();
}

I do not use wxMutexGuiEnter because I am in the main thread for event handling (do I ?).
Nevertheless I tried with it and it does not work better.

I am using SVN 3811 on Win XP SP2 with gcc 3.4.5.

Does anyone have any idea on my problem ?

Thanks,

Dje

Offline TDragon

  • Lives here!
  • ****
  • Posts: 943
    • TDM-GCC
Re: Help on wxWidgets multi threading management
« Reply #1 on: April 05, 2007, 12:01:36 am »
I haven't looked at your ThreadSearch code (yet), but it seems to me that the best thing to do would be to create and update all your wxWidgets GUI objects (such as this cbStyledTextControl) in the main thread (the one where event handling takes place) only, and use wxWidgets' thread-safe event system to handle communication from the worker thread.

For example, create the control, and then start the worker thread. The worker thread starts searching, and then every time it finds a match (or however you've coded it), it posts a custom "MatchFound" event (that derives from wxEvent) to the control, using wxPostEvent. The control's event handler catches the event in the main thread, and updates itself. No mutexes required whatsoever. (Except the ones wxWidgets implements internally.)

Edit 1:
Okay, it looks like you're doing something like that already. Digging deeper...

Edit 2:
Ah, now I get it. You're using the somewhat hack-y method of loading the file to a hidden text control in order to search the text. Digging further...
« Last Edit: April 05, 2007, 12:14:29 am by TDragon »
https://jmeubank.github.io/tdm-gcc/ - TDM-GCC compiler suite for Windows (GCC 9.2.0 2020-03-08, 32/64-bit, no extra DLLs)

Offline dje

  • Lives here!
  • ****
  • Posts: 683
Re: Help on wxWidgets multi threading management
« Reply #2 on: April 05, 2007, 12:14:24 am »
Quote
For example, create the control, and then start the worker thread. The worker thread starts searching, and then every time it finds a match (or however you've coded it), it posts a custom "MatchFound" event (that derives from wxEvent) to the control, using wxPostEvent. The control's event handler catches the event in the main thread, and updates itself. No mutexes required whatsoever. (Except the ones wxWidgets implements internally.)

In fact, I create a hidden cbStyledTextContol to make the search for me because all is already implemented. The wheel has already been invented  :)
Concerning threads safety, I use, as you may have seen, the PostThreadSearchEvent to send search events to the main thread for GUI updates and I protect all accesses to this unique graphical object with wxMutexGuiEnter/Leave.

Note that my code has moved a little to make all kinds of tests to destroy this boring problem.

Dje

Offline eranif

  • Regular
  • ***
  • Posts: 256
Re: Help on wxWidgets multi threading management
« Reply #3 on: April 05, 2007, 12:45:06 am »
Code
wxMutexGuiEnter();
FindInFile(*control, m_FilePaths[i], searchFlags);
wxMutexGuiLeave();

It is not a good idea to do that. The main thread also uses the same mutex before it does anything (look at evtloopcmn.cpp), so if the FinInFile operation is long enough you will hang C::B.
I strongly recommend that you will separate the search logic from the GUI and like TDragon suggested, once you have a result or partial results send an event to update the search result pane to the main thread.

Eran





Offline TDragon

  • Lives here!
  • ****
  • Posts: 943
    • TDM-GCC
Re: Help on wxWidgets multi threading management
« Reply #4 on: April 05, 2007, 01:11:52 am »
Okay, here's my conclusion.

The wxWidgets document has the following to say:
Quote
If you do decide to use threads in your application, it is strongly recommended that no more than one thread calls GUI functions. The thread sample shows that it is possible for many different threads to call GUI functions at once (all the threads created in the sample access GUI), but it is a very poor design choice for anything except an example. The design which uses one GUI thread and several worker threads which communicate with the main one using events is much more robust and will undoubtedly save you countless problems (example: under Win32 a thread can only access GDI objects such as pens, brushes, &c created by itself and not by the other threads).
So it's clear that GUI calls outside the main thread are a bad idea.

Here are the options I see.

Door #1: Try to track down the exact reason for the current issues, and fix it. I wouldn't wish this job on on anyone, except maybe someone intimately familiar with the wxWidgets internals. Multithreading code is hard to debug.

Door #2: A compromise. Move the creation of the hidden control to the main thread. Use a thread-safe message system for all communication with the control, excepting only functions that you've traced through wxScintilla and are sure they make no GUI calls. (FindText() might be safe; I haven't checked thoroughly. LoadFile() is not.) The problem with this approach is that it puts at least a part of the searching work (loading the files) back on the main thread. I don't know how much of an effect this would have on the usability of the C::B interface during a search, but it could be significant -- and I believe the point of this plugin to begin with was to keep the interface usable during searching.

Door #3: The correct fix. Load and search the files without any GUI calls. It means a bit more coding, but not an unreasonable amount to my mind; wxWidgets includes many facilities to make this simple.

If someone can help you out with Door #1, more power to them. I always try to make my code as correct as possible, however, so my recommendation would be Door #3. Whatever you do, I hope you get things worked out satisfactorily.

Cheers,
JohnE / TDM
https://jmeubank.github.io/tdm-gcc/ - TDM-GCC compiler suite for Windows (GCC 9.2.0 2020-03-08, 32/64-bit, no extra DLLs)

Offline jmccay

  • Almost regular
  • **
  • Posts: 202
Re: Help on wxWidgets multi threading management
« Reply #5 on: April 05, 2007, 02:40:42 am »
My experience with threads is limited, but you can not rule out that wxWidgets is trying to get a lock too--somewhere deep inside the dark recesses of the code.  Whereas you be able to get away with this for short code segments which take a very, very small amount of time, I would recommend against it.  You are putting to much coupling (connection) between the GUI and the search code.  The two should be more separated.  There should be a clean line between the two.  This allows you to make changes to either side of the code (GUI or worker) without effecting the unchanged side code.  It should also make the threading issues easier.  As suggested before, using events is the best way to communicate between the two.
jmccay
OS: WinXP, Win98 SE, & sometimes Linux

a little light reading from the wxWidgets 2.6.2 readme: A detailed 2000-page reference manual is supplied in HTML, PDF and Windows Help form: see the docs hierarchy.

Offline dje

  • Lives here!
  • ****
  • Posts: 683
Re: Help on wxWidgets multi threading management
« Reply #6 on: April 05, 2007, 10:13:29 am »
Hi all !

The ThreadSearch plugin main idea is to be able to continue viewing/editing code during a text search (being blocked for 5 minutes is boring and I experienced that too many times). That's why I started this plugin.

I chose to use this graphical control for 3 reasons :
- quite the same strategy as in the 'Search in files'
- allows being consistent with C::B standard search options (case, whole words, beginning of words, reg ex...)
- all is managed by the control, you just have to provide informations and you don't reinvent the wheel.

Nevertheless, I agree that mixing gui calls in the worker thread may be surprising if reasons are not know.
I'll may have a look at Scintilla code to see if it is possible to extract search algorythm quite easily.

But I am very frustrated not understanding why this code does not work.

Concerning the proposed doors :
I tried Door one but I can't find the exit  :(
I am not a fan of door 2 because this plugin looses part of its interest and may be more unpleasant to user.
The third looks the good one but requires more efforts... I'll probably choose this one but I have to improve my knowledge about wxScintilla and wxString possibilities.

Dje

Offline Pecan

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 2750
Re: Help on wxWidgets multi threading management
« Reply #7 on: April 05, 2007, 01:28:02 pm »
You can still use most of the routines in wxScintilla, but without the gui stuff.

Scintilla isn't all that big. I've extracted a number of usefull non-gui routines from it. The sample in ...\src\sdk\wxscintilla\samples\test\ is very useful.

It shows the separation of utilility routines from gui, and someone has already provided a .cbp that you can modify for your environment.

All the find routine's "word boundaries" are built into the non-gui find routines. You dont need a gui to use them.

If you see a routine you need that calls gui, either rip out the calls or (as I've done) write a fake routine to capture it and return a success notice.

wxscintilla.h in the .../src/includes is a nice reference to paste up on the wall (just joking).

But honestly, door 3 isn't going to be as hard as you think. Just move the hidden control to the main thread and call the wxscintilla utility routines on a buffer from the background thread.

As TheDragon :) says, it'll fly... I'm looking forward to your success.
« Last Edit: April 05, 2007, 01:38:37 pm by Pecan »

Offline Pecan

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 2750
Re: Help on wxWidgets multi threading management
« Reply #8 on: April 06, 2007, 02:48:24 am »
You might want to have a look at the wyoEditor.

It looks like it uses a background thread to do FindInFiles. And it's wxScintilla based.

http://wyoguide.sourceforge.net/index.php?page=editor.html
wyoGuide: A guide, a tutorial for developing well-designed cross-platform applications


Offline eranif

  • Regular
  • ***
  • Posts: 256
Re: Help on wxWidgets multi threading management
« Reply #9 on: April 07, 2007, 03:11:39 am »
Hi dje!,

I recently developed a search thread that supports the following:
- match case
- match whole word
- regular expression
- file extensions
- events and easy interface with the main thread

Source code (5 sources + 1 sample):
http://www.eistware.com/st/SearchThread.zip

The code was built and tested on windows using VS20005, so no makefile is provided, but you should be able to complie and run it
easily.

A brief usage:
Code
#include "search_thread.h"

// to use the search thread, we first need to start it ..

//----------------------------------------------------------
// Initialisation
//----------------------------------------------------------
// The search thread is singleton object so there is ohly one instance of it.
SearchThreadST::Get()->Start();

//--------------------------------------------------------------
// Starting new search
//--------------------------------------------------------------
// to perform a search, we create a SearchData object
// and set it with information provided by the user
SearchData data;
data.SetFindString(wxT("something_to_search")); // the find string
data.SetMatchCase(true); // is search case sensetive?
data.SetMatchWholeWord(true); // match whole word?, see also: SetWordChars() function
data.SetRootDir(true); // our search will start here
data.SetRegularExpression(false); // use regular expression?
data.SetExtensions(wxT("*.cpp;*.h;*.hpp;*.hxx")); // look at these file types

SearchThreadST::Get()->SetNotifyWindow( this ); // all events will be fired to 'this'
SearchThreadST::Get()->PerformSearch( data ); // do the search

//-----------------------------------------------------------------------
// Stopping running search
//-----------------------------------------------------------------------
SearchThreadST::Get()->StopSearch();

//------------------------------------------------------------------------
// process results:
//------------------------------------------------------------------------
// SearchThread fires three events:
// wxEVT_SEARCH_THREAD_MATCHFOUND - a match was found
// wxEVT_SEARCH_THREAD_SEARCHEND - search ended
// wxEVT_SEARCH_THREAD_SEARCHCANCELED - search cancelled by user
 
 
// in your code, you need to define 3 event handlers:
// Note: that the events will be sent to the NotifyWindow set earlier
//---------------------------------------------------------------------
EVT_COMMAND(wxID_ANY, wxEVT_SEARCH_THREAD_MATCHFOUND, Frame::OnSearchThread)
EVT_COMMAND(wxID_ANY, wxEVT_SEARCH_THREAD_SEARCHCANCELED, Frame::OnSearchThread)
EVT_COMMAND(wxID_ANY, wxEVT_SEARCH_THREAD_SEARCHEND, Frame::OnSearchThread)

// and the handler ...
void Frame::OnSearchThread(wxCommandEvent &event)
{
if( event.GetEventType() == wxEVT_SEARCH_THREAD_MATCHFOUND)
{
// wxEVT_SEARCH_THREAD_MATCHFOUND returns a SearchResultList
// which is defined as:
// typedef std::list<SearchResult> SearchResultList;
// see SearchResult for more details.
SearchResultList *res = (SearchResultList*)event.GetClientData();
SearchResultList::iterator iter = res->begin();

wxString msg;
for(; iter != res->end(); iter++){
msg.Append((*iter).GetMessage() + wxT("\n"));
}

m_debugWin->AppendText(msg);
delete res;
}
else if(event.GetEventType() == wxEVT_SEARCH_THREAD_SEARCHCANCELED)
{
// wxEVT_SEARCH_THREAD_SEARCHCANCELED sets a cancell message in the GetString() method of the
// the event and nothing more
m_debugWin->AppendText(event.GetString() + wxT("\n"));
}
else if(event.GetEventType() == wxEVT_SEARCH_THREAD_SEARCHEND)
{
// When search ended, a summary object is sent to user:
// The summary contains the number of files scanned and number of total matches found
// a GetMessage() is provided that formats a nice suammry string
SearchSummary *summary = (SearchSummary*)event.GetClientData();
m_debugWin->AppendText(summary->GetMessage() + wxT("\n"));
delete summary;
}
}

//--------------------------------------------------------------
// Clean up
//--------------------------------------------------------------

// before shutdown, it is recommended to stop the SearchThread and free
// its resources:
SearchThreadST::Get()->Stop();
SearchThreadST::Free();


You can use it as is, or just for reference,

HTH,
Eran