Author Topic: rev 7747 question: critical section enter and leave?  (Read 27506 times)

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: rev 7747 question: critical section enter and leave?
« Reply #15 on: February 01, 2012, 03:22:38 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.
What strings are these?
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?
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...)
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.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: rev 7747 question: critical section enter and leave?
« Reply #16 on: February 01, 2012, 05:14:29 pm »
How can you reference the caller in the locker's destructor?
A good reason for the macros, btw. ;-)
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
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);
}

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
No reason, really. Strings are only ever modified from one thread concurrently.
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: rev 7747 question: critical section enter and leave?
« Reply #17 on: February 01, 2012, 05:20:34 pm »
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.
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: rev 7747 question: critical section enter and leave?
« Reply #18 on: February 01, 2012, 05:45:12 pm »
Now with a locker, you automatically and unconditionally clean up in presence of an exception, which includes unlocking.
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.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: rev 7747 question: critical section enter and leave?
« Reply #19 on: February 02, 2012, 01:58:46 am »
Quote
other reason is [...] wxString is not thread safe
No reason, really. Strings are only ever modified from one thread concurrently.
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
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.