I'll put my proposal inside base::RefCounted and see what happens,
I'll
let you know. My proposal can fires as well all the time when
something
is wrong, indeed you can define multiple critical section to monitor
at same time and perform a "single thread ever" check (note I'm not
using the SCOPED_TWCHECK).
class ProducerConsumerQueue { //Non thread safe class
public:
produce(int a) { TWCHECK(PushSection); ... }
int consume() { TWCHECK(PopSection); ... }
private:
...
THREADWARNER(ProduceSection);
THREADWARNER(ConsumeSection);
};
with that controls in place as soon the same thread calls the produce
and the consume,
doesn't matter the time sequence, then you have an assert, all goes
fine if the push
is always called from one thread and pop from another. So if you have
a consumer
and a producer thread and a consumer performs a
ProducerConsumerQueue::produce
by mistake then the assert fires. Basically the first time a thread
calls produce/consume
it "bounds" the method to it. This is actually another use case not
doable with NonThreadSafe.
Gaetano
On Oct 3, 9:42 am, "Adam Barth" <[EMAIL PROTECTED]> wrote:
> Yeah, you're right. What's nice about the current NonThreadSafe is
> that it fires all the time when something is wrong. I got excited
> about this stuff after fixing a couple race conditions from the
> distributed reliability tests that could have been prevented with
> NonThreadSafe.
>
> Adam
>
> On Fri, Oct 3, 2008 at 12:07 AM, Dean McNamee <[EMAIL PROTECTED]> wrote:
> > Adam, this is a good idea, but the problem is we're super super
> > unlikely to actually hit the two code paths at once. So while it
> > would definitely be a bug and we should fix it, for us to actually
> > have the DCHECK fire would be unlikely, since it's a race condition
> > that will most likely always be safe. Getting two cores of a
> > processor to hit a specific increment or decrement at the same time...
> > Even if you did something to slow things down, like sleep in the the
> > methods, it's still not enough really... I guess this sort of stuff
> > just needs to be figured out statically?
>
> > On Fri, Oct 3, 2008 at 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
-~----------~----~----~----~------~----~------~--~---
