14-May-2013 08:33, Heinz пишет:
>> BTW, given recent discussion on memory barriers, I think my previous
>> statement that the mutex does not need to be locked to call notify is
>> probably incorrect.
>

Have to lock it otherwise you have a race condition on a condition variable (wow!).

> Haven't had any issues calling notify outside a synchronized statement,
> even from multiple threads. At least this works under Win32 with my
> producer thread all wrapped inside synchronized(). Only a single
> consumer thread is inside synchronized() but then i have 2 more threads
> making naked calls to notify(). Not a single crash.

Doesn't prove anything, it could happen that you just miss a notification, for instance. Another common case is that it so happens that wait will (with luck) always happen before any of notify and notifications come spaced out in time.

-- 
Dmitry Olshansky

On Monday, 13 May 2013 at 21:04:23 UTC, Juan Manuel Cabo wrote:

> There is one thing that should definitely added to the documentation, and that is what happens when one issues a notify while the thread hasn't yet called Condition.wait().

I can confirm that under Win32 calling notify() before wait()
internally signals the condition and then calling wait() returns
immediately and actually does not wait. This is the expected
behavior and is actually how Win32 events work.

On Tuesday, 14 May 2013 at 08:58:31 UTC, Dmitry Olshansky wrote:

> Have to lock it otherwise you have a race condition on a condition variable (wow!).

Ok, i'll lock it just in case. It also makes me feel my code is
more robust. This will do right?

...
synchronized(cond.mutex)
     cond.notify();
...

My internal bool variable that affects the condition (the one
that decides if the consumer thread should wait) must be setable
at any moment by any thread so i leave it outside the lock. Also,
after setting this variable i immediately call notify() with
mutex unlocked. That's why it is working i think.

> Doesn't prove anything, it could happen that you just miss a notification, for instance. Another common case is that it so happens that wait will (with luck) always happen before any of notify and notifications come spaced out in time.

True, i guess my code work 100% as needed because of its
implementation, in my case it doesn't matter if i miss a
notification because the result of that notification gets
evaluated in the next loop inside a while, but still most
probably as you say wait() is always happening before a
notification.
A classic producer-consumer program must lock both calls, i do
understand that.

I'm doing this from my phone so please bear with me.

You use a mutex in combination with a condition variable so you can check the state of something to determine if waiting is necessary. So the classic producer/consumer would be something like:

T get() {
    shnchronized(c.mutex) {
        while (q.isEmpty)
            c.wait();
        return q.take();
    }
}

void put(T val) {
    synchronized (c.mutex) {
        q.add(val);
        c.notify();
    }
}

You can emulate a Win32 event that stays flipped by checking/setting a bool or int inside the mutex.

14-May-2013 20:09, Heinz пишет:
> On Monday, 13 May 2013 at 21:04:23 UTC, Juan Manuel Cabo wrote:
>
>> There is one thing that should definitely added to the documentation,
>> and that is what happens when one issues a notify while the thread
>> hasn't yet called Condition.wait().
>
> I can confirm that under Win32 calling notify() before wait()
> internally signals the condition and then calling wait() returns
> immediately and actually does not wait. This is the expected
> behavior and is actually how Win32 events work.


> On Tuesday, 14 May 2013 at 08:58:31 UTC, Dmitry Olshansky wrote:
>
>> Have to lock it otherwise you have a race condition on a condition
>> variable (wow!).
>
> Ok, i'll lock it just in case. It also makes me feel my code is
> more robust. This will do right?
>
> ...
> synchronized(cond.mutex)
>       cond.notify();
> ...
>
> My internal bool variable that affects the condition (the one
> that decides if the consumer thread should wait) must be setable
> at any moment by any thread so i leave it outside the lock.

Wrong. Before setting it from some other thread you should grab the lock. See Sean's example.

> Also,
> after setting this variable i immediately call notify() with
> mutex unlocked. That's why it is working i think.
>


-- 
Dmitry Olshansky

On Tue, 14 May 2013 04:58:27 -0400, Dmitry Olshansky <dmitry.olsh@gmail.com> wrote:

> 14-May-2013 08:33, Heinz пишет:
>>> BTW, given recent discussion on memory barriers, I think my previous
>>> statement that the mutex does not need to be locked to call notify is
>>> probably incorrect.
>>
>
> Have to lock it otherwise you have a race condition on a condition variable (wow!).

No, the issue would be reordering (is that possible in this case?).  The actual signaling of the condition would not require the lock, but you still need to lock to send the message (e.g. set the boolean) or you will definitely have issues.

But since you have to lock anyway, signaling while holding the lock, or while being outside the lock isn't really a difference.

Maybe I'm wrong...

>> Haven't had any issues calling notify outside a synchronized statement,
>> even from multiple threads. At least this works under Win32 with my
>> producer thread all wrapped inside synchronized(). Only a single
>> consumer thread is inside synchronized() but then i have 2 more threads
>> making naked calls to notify(). Not a single crash.
>
> Doesn't prove anything, it could happen that you just miss a notification, for instance. Another common case is that it so happens that wait will (with luck) always happen before any of notify and notifications come spaced out in time.

I don't see how you could miss a notification, can you explain further?

-Steve

Guys, this is a precise example of what i'm trying to do. You'll notice that there're 2 ways of waking up the consumer:

/////////////////////////////////////////////////////////////////////
Condition cond; // Previously instantiated.
bool loop = false; // This variable determine if the consumer should take the next iteration or wait until a thread calls SetLoop(true).
Object my_variable; // A value used for comunicating producer and consumer. It's not a prerequisite to be set for an iteration to happen.

// This function is called by multiple threads.
void SetLoop(bool val)
{
	if(val != loop)
	{
		loop = val;
		if(loop)
			cond.notify(); // Wake up consumer in case it was waiting.
	}
}

// This function is called by multiple threads.
bool GetLoop()
{
	return loop;
}

void MyConsumer()
{
	while(true)
	{
		synchronized(cond.mutex)
		{
			if(loop == false)
				cond.wait(); // Wait for producer or SetLoop(true).
			
			if(my_variable !is null)
			{
				/*
				...
				Call private function 1. SetLoop(true/false) might be called here.
				...
				*/
				my_variable = null; // Lock is useful here for setting my_variable.
			}
			
			// These conditions are intentionally placed here.
			if(loop == false)
				continue; // Jump to next iteration. Please note that cond.mutex gets released and reaquired in the next iteration.
			else
				loop = false; // Reset waiting on every iteration. Can be modified by private function 2 below.
			
			/*
			...
			Call private function 2. SetLoop(true/false) might be called here.
			...
			*/
		}
	}
}

void MyProducer()
{
	while(true)
	{
		if(/* Some other condition */)
		{
			synchronized(cond.mutex) // Lock is useful here for setting my_variable.
			{
				my_variable = /* Some value */;
				cond.notify(); // Wake up consumer in case it was waiting.
			}
		}
	}
}
/////////////////////////////////////////////////////////////////////

Wouldn't wrapping all the lines in "SetLoop()" inside a "synchronized(cond.mutex)" statement produce a deadlock because it might be called by private functions 1 or 2? Should i only wrap cond.notify() or is it of no meaning in this case? Like this:

void SetLoop(bool val)
{
	if(val != loop)
	{
		loop = val;
		if(loop)
		{
			synchronized(cond.mutex)
				cond.notify(); // Wake up consumer in case it was waiting.
		}
	}
}

Synchronization of variable "loop" doesn't seem important in this case. Do you still recommend to use an extra mutex to set it? like this:

void SetLoop(bool val)
{
	synchronized(extra_mutex)
	{
		if(val != loop)
		{
			loop = val;
			if(loop)
			{
				synchronized(cond.mutex)
					cond.notify(); // Wake up consumer in case it was waiting.
			}
		}
	}
}

void MyConsumer()
{
	//...
	if(loop == false)
		continue;
	else
	{
		synchronized(extra_mutex)
			loop = false;
	}
	//...
}

Thank you guys so much for your time and your expertise.

On Tue, 14 May 2013 13:59:51 -0400, Heinz <thor587@gmail.com> wrote:

> Guys, this is a precise example of what i'm trying to do. You'll notice that there're 2 ways of waking up the consumer:

2 things:

1. D mutex locks are re-entrant.  That is, you can do:

synchronized(mutex)
{
   synchronized(mutex)
   {
      ...
   }
}

and all is fine.

2. The condition logic in your code is wrong.  The condition/mutex protects the messages being sent to the consumer.

It seems that you have two messages (which is OK):

 - my_variable is present, needs to be processed
 - Run function 2 (flag loop == run function 2), set from inside the consumer, and some undisclosed threads.  I find this logic rather dangerous, but it is your app, I have no idea of the real meaning :)

So I would do it like this (names TBD based on your real use case)

void SetLoop(bool yesOrNo)
{
   synchronized(cond.mutex)
   {
       if((loop = yesOrNo) == true)
            cond.notify();
   }
}

void MyConsumer()
{
	while(true)
	{
                Object _my_variable = null;
		synchronized(cond.mutex)
		{
			while(!loop && my_variable is null) // always wait for *real* messages to be set
				cond.wait();
			_my_variable = my_variable;
			my_variable = null; // consume
                }

		// process local consumed copy of _my_variable *outside* of lock
		if(_my_variable !is null)
		{
			/*
			...
			Call private function 1. SetLoop(true/false) might be called here.
			...
			*/
		}
			
		// These conditions are intentionally placed here.
		synchronized(cond.mutex)
		{
			if(loop == false)
				continue; // Jump to next iteration. Please note that cond.mutex gets released and reaquired in the next iteration.
			else
				loop = false; // Reset waiting on every iteration. Can be modified by private function 2 below.
		}
			
		/*
		...
		Call private function 2. SetLoop(true/false) might be called here.
		...
		*/
	}
}

Then everything should be good.  I'd also add a "SetMyVariable" function too, akin to SetLoop.

-Steve

On May 14, 2013, at 9:09 AM, Heinz <thor587@gmail.com> wrote:

> On Monday, 13 May 2013 at 21:04:23 UTC, Juan Manuel Cabo wrote:
> 
>> There is one thing that should definitely added to the documentation, and that is what happens when one issues a notify while the thread hasn't yet called Condition.wait().
> 
> I can confirm that under Win32 calling notify() before wait()
> internally signals the condition and then calling wait() returns
> immediately and actually does not wait. This is the expected
> behavior and is actually how Win32 events work.

A Win32 event can be simulated basically like so:

class Event {
    Condition c;
    bool signaled = false;

    this() {
        c = new Condition(new Mutex);
    }

    void wait() {
        synchronized (c.mutex) {
            while (!signaled)
                c.wait();
            signaled = false;
        }
    }

    void notify() {
        synchronized (c.mutex) {
            signaled = true;
            c.notify();
        }
    }
}

auto e = new Event;

T get() {
    while (true) {
        e.wait();
        // A -- race here
        synchronized (m) {
            if (!m.isEmpty())
                return m.take();
        }
    }
}

void put(T val) {
    synchronized(m) {
        m.add(val);
    }
    e.notify();
}

You'll notice the redundancy here though to combat the race at point A.  Generally, you really want to avoid the Win32 model and use the mutex/condition pair directly with your container or whatever.


> On Tuesday, 14 May 2013 at 08:58:31 UTC, Dmitry Olshansky wrote:
> 
>> Have to lock it otherwise you have a race condition on a condition variable (wow!).
> 
> Ok, i'll lock it just in case. It also makes me feel my code is more robust. This will do right?
> 
> ...
> synchronized(cond.mutex)
>     cond.notify();
> …

Yep.


> My internal bool variable that affects the condition (the one that decides if the consumer thread should wait) must be setable at any moment by any thread so i leave it outside the lock. Also, after setting this variable i immediately call notify() with mutex unlocked. That's why it is working i think.

I don't understand what you mean here.  If the variable is protected by a lock it can still be set by any thread at any time.  Just only one thread at a time.  Doing a lock-free write of the variable basically just means that the variable will probably be set eventually, which is rarely what you actually want.

On May 14, 2013, at 10:59 AM, Heinz <thor587@gmail.com> wrote:

> Guys, this is a precise example of what i'm trying to do. You'll notice that there're 2 ways of waking up the consumer:
> 
> /////////////////////////////////////////////////////////////////////
> Condition cond; // Previously instantiated.
> bool loop = false; // This variable determine if the consumer should take the next iteration or wait until a thread calls SetLoop(true).
> Object my_variable; // A value used for comunicating producer and consumer. It's not a prerequisite to be set for an iteration to happen.

Let's back up a minute.  Can you be more specific about what you're trying to do?  I think you shouldn't need to use the "loop" var at all, but I'm not entirely sure.  Also, loop and my_variable will be thread-local given how they're declared.

On Tuesday, 14 May 2013 at 17:59:55 UTC, Heinz wrote:
> Guys, this is a precise example of what i'm trying to do. You'll notice that there're 2 ways of waking up the consumer:
>
> /////////////////////////////////////////////////////////////////////
> Condition cond; // Previously instantiated.
> bool loop = false; // This variable determine if the consumer should take the next iteration or wait until a thread calls SetLoop(true).
> Object my_variable; // A value used for comunicating producer and consumer. It's not a prerequisite to be set for an iteration to happen.

You also need to declare Condition as __gshared. If you don't,
each thread will see a different Condition object or null,
because by default, every D global variable goes to Thread Local
Storage.

Any variables meant to be shared between threads need to be
either '__gshared' or 'shared'. You could also declare other
shared data as 'shared', but I don't think that's what you want
here.

To list all global variables not declared as __gshared, you can
compile using the -vtls switch.

--jm