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

rev 7747 question: critical section enter and leave?

<< < (4/4)

ollydbg:

--- Quote from: MortenMacFly on February 01, 2012, 03:05:51 pm ---
--- Quote from: ollydbg on February 01, 2012, 02:56:56 pm ---He explained to me that the reason is "wxString is not thread safe", this means some wxString's contents may be shared in both TreeA and TreeB, as the wxString(wx2.8.x) can not protect their reference counter in multiply threads accessing.

--- End quote ---
What strings are these?

--- End quote ---
You mean what strings whose reference counting is thread safe? std::sting and std::wstring under gcc/mingw are this kind of strings. (the std::string under Visual c++ 05?08? or later even do not have reference count feature, so if you copy them, you have a deep copy, that's not a big deal in modern CPUs)

--- Quote ---Just the one in the tokens, or are there more?

--- End quote ---
There are many wxStrings in TokensTree class.



--- Quote ---Why don't we simply replace them with "something" thread-safe? (One reason I could think of not so easy with something else than wxString is encoding conversion btw...)

--- End quote ---
I don't know the reason, it looks like we should DO this. Though the next generation of wxWidgets (2.9.x) have internally use std::string/wstring, but it have another issue. This "another issue" is:

Under Linux, the wxString is internally a std::string, and all the strings are recorded in UTF8 format.
Under Windows, the wxString is internally a std::wstring, and all the strings are recored in UTF16 format(the name under Windows we call it UNICODE format or wide char format)

You see, under Linux, we may have potential problems to access a wxChar(not sure whether under wx2.9.x we can still use wxChar) through an index value, because it use a variable length encoding.

thomas:

--- Quote from: MortenMacFly on February 01, 2012, 02:40:14 pm ---
--- Quote from: ollydbg on February 01, 2012, 02:22:03 pm ---How can you reference the caller in the locker's destructor?

--- End quote ---
A good reason for the macros, btw. ;-)

--- End quote ---
Not really  ;)
First, being agnostic of a better solution, you can get this very effect by wrapping the locker's constructor in a macro and forwarding the constants. You'll still have a macro, but you'll retain exception safety and avoid generating more deadlocks.

Second, it's not necessary at all, since gcc allows you to query a function's return address without any hassle. addr2line translates the address back. I'm using this technique in one of my projects to track where an exception that made it to the outermost handler came from, this works like a charm.


--- Quote ---There are too many lockers [...] access to the TokensTree from either GUI or working thread
--- End quote ---
Ok, this is a reason for one locker, but not for many. Normally, the high-level flow of a worker would be something like this:

--- Code: ---while(get_next_file())
{
    while(token = parse())
        my_list.push_back(token);

    do{
        locker(lock);
        for(auto t : my_list) ParseTree.add(t);
    }while(0);
}
--- End code ---

Of course you wouldn't just store tokens in a list (makes the example simpler, though). But nothing hinders you to build your own parse sub-tree and have a function that merges a sub-tree into the main tree, that's just an implementation detail.


--- Quote ---other reason is [...] wxString is not thread safe
--- End quote ---
No reason, really. Strings are only ever modified from one thread concurrently.

thomas:
To explain what I mean with "avoid generating more deadlocks", the scary thing about these macros is that they may create deadlocks which would not otherwise be there.

If an exception is thrown between lock and unlock, you have a deadlock (in addition to any you might have had before). Even if the exception is gracefully handled (which admittedly is often not the case anyway).

Now with a locker, you automatically and unconditionally clean up in presence of an exception, which includes unlocking.

MortenMacFly:

--- Quote from: thomas on February 01, 2012, 05:20:34 pm ---Now with a locker, you automatically and unconditionally clean up in presence of an exception, which includes unlocking.

--- End quote ---
Look: What I am trying to do it TO GET RID of so many lockers. But to do so, I must know how the message flow goes. So all this macro stuff is just there for debugging.
In the end, you can easily adjust the macro later at any time to use a locker object - simply by changing the ENTER/LOCK macro to create an object and changing the LEAVE/UNLOCK macro to turn into nothing. Easy, isn't it?

Look at r7763 (cclogger.h) - I've done it similar to detect dead locks in none-debug mode.

Again: The final goal is t remove this stuff, not to keep it.

ollydbg:

--- Quote from: thomas on February 01, 2012, 05:14:29 pm ---
--- Quote ---other reason is [...] wxString is not thread safe
--- End quote ---
No reason, really. Strings are only ever modified from one thread concurrently.

--- End quote ---
I'm not saying that two thread are accessing one string instance, I'm saying that two thread are accessing two instances. Like stringA and stringB, but maybe, stringB is copied from stringA, they internally share contents. E.g. A parserthread collect a Token(stringA), and store it in TokensTree(stringB in TokensTree).

As stringA and stringB are reference counted, if reference counter is thread safe, then the above situation is OK. But wxString's reference counter is not thread safe.

This is the article talking about this kind of issue and performanceOptimizations That Aren't (In a Multithreaded World), currently gcc use the atomic variables and atomic functions to protect the reference counter in std::string/wstings. See:C/C++ reference counting with atomic variables and gcc - Alex on Linux

Navigation

[0] Message Index

[*] Previous page

Go to full version