I uploaded the CL http://codereview.chromium.org/6258, and applied it
at the base::RefCountedBase, all tests are passing, I tried to change
base
class for MessagePump to base::RefCounted and during the tests
the assert fires as expected, I'm confident that as soon RefCounted
is used instead of RefCountedThreadSafeBase somewhere then the
assert will fire.
The CL is not ready indeed I still use the two atomic operations:
__sync_bool_compare_and_swap
__sync_fetch_and_xor <= I use this to assign 0 to valid_thread_id_
I have to look at atomicops.h to figure out how to replace those.
Another open issue on the CL submitted is the usage of assert instead
of DCHECK, as soon I include "base/logging.h" then I have the macro
LOG in collision with the WebKit one.
Gaetano
On Oct 3, 2:41 am, "Adam Barth" <[EMAIL PROTECTED]> wrote:
> Gaetano,
>
> I think I understand what you're saying. Let me try to say it in my
> own words and you can tell me if I've gotten it right.
>
> You might have an object that is not designed to be thread safe, but
> can be used safely if only one of its methods is running at any given
> time. So you want to DCHECK that this actually occurs. NonThreadSafe
> is designed for object that are only used on a single thread so it
> doesn't match this use case.
>
> You know, this matches a problem that we've been having but haven't
> found a good solution for. Many objects inherit from
> base::RefCounted, which is not designed to be thread safe (there is
> base::RefCountedThreadSafe for that, but its a bit slower because it
> uses atomic increment, etc). We tried to make RefCounted inherit from
> NonThreadSafe, but it turned out there were a lot of cases of a
> RefCounted object being created on one thread and then handed over to
> another thread. I examined a bunch of these by hand, and they were
> all perfectly safe.
>
> What we'd really like is a mechanism like you've described for
> checking that AddRef() and Release() are never running at the same
> time (which would be a bad race leading to memory leaks or memory
> corruption depending on how the race turns out.)
>
> As a proof-of-concept, can you test your approach on RefCounted and
> see if you can find race conditions? If you can find a few races this
> way, we can fix them and figure out how to integrate this DCHECK into
> our code base and make it work cross-platform.
>
> Adam
>
> On Thu, Oct 2, 2008 at 4:14 PM, Gaetano Mendola <[EMAIL PROTECTED]> wrote:
>
> > On Oct 2, 10:59 pm, "Adam Barth" <[EMAIL PROTECTED]> wrote:
> >> Can you describe a use-case that is missing from NonThreadSafe?
> >> NonThreadSafe has already found lots of race conditions for us. I'm
> >> definitely interested in ways to improve it.
>
> > If I'm not wrong, with NonThreadSafe what you can check is that
> > some methods (those in where DCHECK(CalledOnValidThread()) is
> > inserted)
> > are executed from same thread that build the class.
>
> > On my first message of this thread you can not cover with
> > NonThreadSafe
> > (at least I don't see how) the Case #1 (unless the thread using the
> > class is
> > the same that build it) or the Case #3.
>
> > Some real examples:
>
> > Two threads interact with a class Queue not thread safe:
>
> > class IntQueue { //Non thread safe class
> > public:
> > push(int a) { ... }
> > int pop() { ... }
> > private:
> > ...
> > };
>
> > Main thread builds a IntQueue instance, and two different threads get
> > a reference to that instance.
> > With NonThreadSafe, unless I'm missing something, you can not do the
> > same thing is possible to
> > do with my proposal:
>
> > class IntQueue { //Non thread safe class
> > public:
> > push(int a) { SCOPED_TWCHECK(PushPopSection); ... }
> > int pop() { SCOPED_TWCHECK(PushPopSection); ... }
> > private:
> > ...
> > THREADWARNER(PushPopSection);
> > };
>
> > in this way the main thread can build an instance of IntQueue, and the
> > other two threads can
> > call the push/pop if they are correctly sincronized with a shared
> > mutex;
> > basicaly the THREADWARNER in this example checks that the mutex is in
> > place and most
> > of all it's working as expected.
>
> > Gaetano
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Chromium-dev" group.
To post to this group, send email to chromium-dev@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/chromium-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
