Developer forums (C::B DEVELOPMENT STRICTLY!) > Plugins development

Help on wxWidgets multi threading management

(1/2) > >>

dje:
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.
--- End quote ---

My worker thread uses the control in this way:

--- Code: ---wxMutexGuiEnter();
FindInFile(*control, m_FilePaths[i], searchFlags);
wxMutexGuiLeave();
--- End code ---

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();
}
--- End code ---

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

TDragon:
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...

dje:

--- 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.)
--- End quote ---

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

eranif:

--- Code: ---wxMutexGuiEnter();
FindInFile(*control, m_FilePaths[i], searchFlags);
wxMutexGuiLeave();
--- End code ---

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

TDragon:
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).

--- End quote ---
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

Navigation

[0] Message Index

[#] Next page