Jump to content
  • Advertisement
Sign in to follow this  
Solid_Spy

c++11 Multithreading not working! Please help!

This topic is 2135 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

hello, i've been trying to synchronize two threads, however once one runs, the other one never continues. I've been following tutorials online, but they don't seem to help. Can someone please tell me what i'm doing wrong?

 

here's the code:

#include <iostream>
#include <Windows.h>
#include <iomanip>
#include <thread>
#include <mutex>
#include <chrono>
#include <condition_variable>

std::mutex mutex1;
std::mutex mutex2;
std::condition_variable notReady1;
std::condition_variable notReady2;

void startThread1();
void startThread2();

int main()
{
	std::cout << "Hello world!\n";

	std::thread thread1(startThread1);
	std::thread thread2(startThread2);

	thread1.join();
	thread2.join();

	Sleep(1000);

	return 0;
}

void startThread1()
{
	SetThreadAffinityMask(GetCurrentThread(), 0x01);

	std::unique_lock<std::mutex> lock(mutex1);

	do
	{
		std::cout << "1";

		Sleep(1);

		notReady2.notify_one();
		notReady1.wait(lock);
	}while(true);
}

void startThread2()
{
	SetThreadAffinityMask(GetCurrentThread(), 0x02);
	std::unique_lock<std::mutex> lock2(mutex2);

	while(1)
	{
		notReady2.wait(lock2);

		std::cout << "2\n";

		Sleep(1);

		notReady1.notify_one();
	}
}

I'm trying to get an output like : 12

12

12...

 

but it's not working. It freezed at 1.

Share this post


Link to post
Share on other sites
Advertisement

It is customary in these situations to wait and lock on the same mutex. That way you never miss a notification.

This. You're supposed to use the same mutex for condition variables.

 

That might seem weird because you're locking on the same mutex. I suggest you read this (yes, it's talking about pthreads, but the idea is pretty much the same with C++ threads and condition variables, that's why you have to wait on a lock (and the lock should share a mutex between the two threads)).

Share this post


Link to post
Share on other sites

You have a race condition.

Thread 1 can notify thread 2  before thread 2 ever starts waiting.

So thread 2 will continue waiting forever because thread1 notified it when it wasn't waiting.

 

It is customary in these situations to wait and lock on the same mutex. That way you never miss a notification.

Could I also use a while loop to constantly notify the thread until it is notified, then exit the loop? I saw something like that before, but is it efficient?

Edited by Solid_Spy

Share this post


Link to post
Share on other sites

It works! I used a while loop to check if the thread was being accessed. Here's the code:

#include <iostream>
#include <Windows.h>
#include <iomanip>
#include <thread>
#include <mutex>
#include <chrono>
#include <condition_variable>

std::mutex mutex1;
std::mutex mutex2;
bool thread1Notified;
bool thread2Notified;
std::condition_variable notReady1;
std::condition_variable notReady2;

void startThread1();
void startThread2();

int main()
{
	std::cout << "Hello world!\n";

	thread1Notified = false;
	thread2Notified = false;

	std::thread thread1(startThread1);
	std::thread thread2(startThread2);

	thread1.join();
	thread2.join();

	Sleep(1000);

	return 0;
}

void startThread1()
{
	SetThreadAffinityMask(GetCurrentThread(), 0x01);

	std::unique_lock<std::mutex> lock(mutex1);

	do
	{
		std::cout << "1";

		Sleep(1);
		while(!thread2Notified)
		{
			notReady2.notify_one();
			Sleep(1);
		}
		thread2Notified = false;

		notReady1.wait(lock);
		thread1Notified = true;
	}while(true);
}

void startThread2()
{
	SetThreadAffinityMask(GetCurrentThread(), 0x02);
	std::unique_lock<std::mutex> lock2(mutex2);

	while(1)
	{
		notReady2.wait(lock2);
		thread2Notified = true;

		std::cout << "2\n";

		Sleep(1);

		while(!thread1Notified)
		{
			notReady1.notify_one();
			Sleep(1);
		}
		thread1Notified = false;

	}
}

However, I want to know if what i'm doing is correct.

Edited by Solid_Spy

Share this post


Link to post
Share on other sites

So basically you ignored everyone trying to tell you that your code is fundamentally wrong in using separate mutexes (which completely breaks the whole notify/wait mechanism) and instead added your own busy waiting loop, a lot of Sleeps and are now spamming notifications, which have become kind of pointless anyway?

 

The idea is to lock the (common) mutex and check for the condition before you start waiting (in a loop, because of spurious wake ups). Locking the mutex and using the same mutex for one condition is important, else you might get the notify exactly between checking and waiting, which would again result in waiting forever. Why is this condition not part of the condition variable? Because it isn't always a simple bool that is directly tied to it. The condition can (and often is) something like "!queue.empty()".

 

Besides, the idea of notify_one is that exactly one waiting thread will wake up. By spamming notify_one, you could as well rename it to notify_a_bunch.

Share this post


Link to post
Share on other sites

So basically you ignored everyone trying to tell you that your code is fundamentally wrong in using separate mutexes (which completely breaks the whole notify/wait mechanism) and instead added your own busy waiting loop, a lot of Sleeps and are now spamming notifications, which have become kind of pointless anyway?

 

The idea is to lock the (common) mutex and check for the condition before you start waiting (in a loop, because of spurious wake ups). Locking the mutex and using the same mutex for one condition is important, else you might get the notify exactly between checking and waiting, which would again result in waiting forever. Why is this condition not part of the condition variable? Because it isn't always a simple bool that is directly tied to it. The condition can (and often is) something like "!queue.empty()".

 

Besides, the idea of notify_one is that exactly one waiting thread will wake up. By spamming notify_one, you could as well rename it to notify_a_bunch.

I didn't know what they meant by using the same mutex. I tried using one global mutex but that didn't seem to work. I wasn't sure if I was doing it right.

Share this post


Link to post
Share on other sites

Explicitly calling sleep functions is considered bad practice.

Well is there any other way to stop the thread from hogging the cpu?

Edited by Solid_Spy

Share this post


Link to post
Share on other sites

Here... Just read over this carefully. Also, I really do hope you read that link I gave earlier. Edit: I did if instead of while... wait() might spuriously wake up. It's fixed now.

#include <iostream>
#include <thread>
#include <mutex>
#include <condition_variable>
 
// Each condition variable has its own mutex. The mutex is meant to protect
// the condition variable itself (and any potentially shared data).
 
// Also, I suggest you read:
// http://en.cppreference.com/w/cpp/thread/condition_variable_any/notify_one
// "The [thread calling notify_one()/notify_all()] does not need to hold the
// lock on the same mutex as the one held by the waiting thread(s); in fact
// doing so is a pessimization, since the notified thread would immediately
// block again, waiting for the notifying thread to release the lock.
 
// Also, there's a potential problem. One thread may notify_one() *before*
// the other thread starts to wait(), which means the other thread will
// miss the signal and hang indefinitely waiting for it to come again.
// For this reason, a lot of people will add a boolean flag and check it
// before locking and calling wait(), as it indicates the signal already
// happened and they shouldn't lock and wait().
 
std::mutex mutex1;
std::condition_variable notReady1;
bool do1 = false;
 
std::mutex mutex2;
std::condition_variable notReady2;
bool do2 = false;
 
void startThread1();
void startThread2();
 
int main()
{
    std::cout << "Hello world!\n";
 
    do1 = true;
 
    std::thread thread1(startThread1);
    std::thread thread2(startThread2);
 
    thread1.join();
    thread2.join();
 
    return 0;
}
 
void startThread1()
{
    for (int i = 0; i < 10; ++i)
    {
        std::cout << "1";
 
        do1 = false;
        do2 = true;
        notReady2.notify_one();
 
        while (!do1)
        {
            std::unique_lock<std::mutex> lock1(mutex1);
            notReady1.wait(lock1);
        }
    }
}
 
void startThread2()
{
    for (int i = 0; i < 10; ++i)
    {
        while (!do2)
        {
            std::unique_lock<std::mutex> lock2(mutex2);
            notReady2.wait(lock2);
        }
 
        std::cout << " 2" << std::endl;;
 
        do2 = false;
        do1 = true;
        notReady1.notify_one();
    }
}
Edited by Cornstalks

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!