Developer forums (C::B DEVELOPMENT STRICTLY!) > CodeCompletion redesign

F() function is not thread safe?

<< < (3/5) > >>

oBFusCATed:

--- Quote from: thomas on January 10, 2014, 12:18:34 pm ---But I'm OK if you think that returning by value is a better choice, even if it does create a copy.

--- End quote ---
As far as I'm concerned (and probably the standard) returning by value is the only option if you're not using a static or global caching variable.


--- Quote from: thomas on January 10, 2014, 12:18:34 pm ---Feel free to choose whatever makes you feel most comfortable.

--- End quote ---
I don't intend to do anything about this function, I'm just commenting your patch:)

thomas:
Well, the standard is totally fine with returning a const reference to a local variable, since this extends the lifetime of the local to the lifetime of the reference.

But like I said, I'm fine with anything.

I seem to be kind of unable to commit at this time (or using wrong password/account/whatever -- in any case it doesn't work?), so someone else needs to decide what to do and commit :)

oBFusCATed:

--- Quote from: thomas on January 10, 2014, 10:29:38 pm ---Well, the standard is totally fine with returning a const reference to a local variable, since this extends the lifetime of the local to the lifetime of the reference.

--- End quote ---
You are so wrong here....

See an example:

--- Code: ---#include <stdio.h>
#include <string>

const std::string & func()
{
std::string temp("value");
return temp;
}

int main()
{
std::string s=func();
return 0;
}

--- End code ---


--- Code: --- g++ -Wall -Wextra return_ref.cpp
return_ref.cpp: In function ‘const string& func()’:
return_ref.cpp:6:14: warning: reference to local variable ‘temp’ returned [enabled by default]

--- End code ---

http://herbsutter.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-const/

ollydbg:

--- Quote from: thomas on January 10, 2014, 12:18:34 pm ---
What worries me more than those micro-optimizations is code like this:

--- Code: ---void TextCtrlLogger::Append(const wxString& msg, Logger::level lv)
{
    if (!control)
        return;

    ::temp_string.assign(msg);
    ::temp_string.append(_T("\n"));

    if (lv == caption)
    {
        control->SetDefaultStyle(style[info]);
        control->AppendText(::newline_string);

        control->SetDefaultStyle(style[lv]);
        control->AppendText(::temp_string);

        control->SetDefaultStyle(style[spacer]);
        control->AppendText(::newline_string);
    }
    else
    {
        control->SetDefaultStyle(style[lv]);
        control->AppendText(::temp_string);
    }
}

--- End code ---

The F function as it was is kind of "semi threadsafe" insofar as all modules that were supposed to use it (mostly compiler, and maybe project mgr) run at most one thread per module. Now the TextCtrlLogger such as being used for the debug log on the other hand is definitely thread-unsafe, but I don't even understand why it uses the global here at all. Appending a newline would make much more sense on a plain normal local variable, no?

--- End quote ---

The function:

--- Code: ---Manager::Get()->GetLogManager()->DebugLog()
Manager::Get()->GetLogManager()->Log()

--- End code ---
are totally thread un-safe, so they can only be used in the main GUI thread.

So, if you have debug messages to be sent from a worker thread, you need to implement your own thread safe log function.

BTW: Is it possible to use wxLog functions in wxWidgets, such as wxLogMessage, since they can redirect the messages to a wxTextCtrl, and I see in wx document, they are thread safe.

ollydbg:
Let me give some conclusion:
1,  F() function can be optimized to becomes thread safe by returning a const wxString (returning a const wxString reference gives build error on my system just as OBF said)

2,  Our DebugLog() and Log() functions are not thread safe, because it will directly operate on the wxTextCtrl.

Here is my solution:
1, I don't intend to fix the F() function, instead I will add some comments around the F(). Reuse the global wxString should be much faster if we only call F() and DebugLog() in the main GUI thread.
2, I will create an alternative SAFE_F() function (which return a const wxString). It is used only in CodeCompletion plugin.
3, CodeCompletion plugin already implements its own thread safe logger functions (those functions can be called in multiply threads) by posting the log events from any threads to the main GUI.

EDIT: I will commits the changes if there is no further objections.

Navigation

[0] Message Index

[#] Next page

[*] Previous page