Shared variable between thread(ISR) and main()

[Jonathan100]

Hello folks,

I want to create a variable that will be shared between a thread (ISR) and main()
I am working on embedded environment on linux os

Basically, my thread consists of:

int flag = 0;
My Thread(){ 
if (flag == 1){
	printf("Error: %d\n\r",intr_counter);
	flag=0;
	}
else  flag = 1;
}

My main():

 extern int flag;
while(1){
if (flag==1) {
...
...
flag = 0;
}
}

As you can see, in the thread i make the flag to be '1', and then in main() while loop - if flag is '1' i do certain task and then set flag to be '0'

I found out couple of problems - the code isn't executes properly - hence I suspect it is an issue of synchronization.

What do you think?
In case this is the problem, what method can I use to sync 'flag' variable?

[salem c]

After setting flag to 0, how quickly does it get set back to 1 ?
If that is very quick, main might not see it.

Also, make it an atomic type.
https://en.cppreference.com/w/c/thread

> Basically
Perhaps try 'actually', with some code that can be compiled and run.
Paraphrasing a bunch of stuff can easily miss very important details when dealing with threads and sync issues.

[George P]

More often than not what you think and believe to be "the problem" isn't where the fault lies.

Our crystal balls are busted, so our ability to divine what you are doing is nonexistent.

A short, self contained, compileable and correct example of what you are trying to do would go a long way to helping us be able to help you. Throwing up tiny tidbits of code snippets with little to no context of what you are doing doesn't really work.

http://www.sscce.org/

[kigar64551]

Reading or writing "shared" data, such as a global variable, from concurrent threads will not work properly, unless every read or write access to the "shared" data is protected by a Mutex (mutual exclusion lock) or, maybe, by an R/W Lock (readers/writer lock).

Without proper synchronization, there is no gurantee that thread #2 will "see" the updated value written by thread #1 (anytime soon).

Also, without proper synchronization, one thread may "see" partially updated "garbage" data, while the other thread is currently writing the new data to the shared memory location. Even though that is unlikely to happen with just a single int value, you must keep it in mind!

Change your code to:

extern int flag;
extern pthread_mutex_t mutex;

int main()
{
    int current_value;

    for(;;)
    {
        pthread_mutex_lock(&mutex);
        current_value = flag;
        pthread_mutex_unlock(&mutex);

        if (current_value == 1)
        {
            pthread_mutex_lock(&mutex);
            flag = 0;
            pthread_mutex_unlock(&mutex);
        }
    }
}

int flag = 0;
pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;

void my_thread()
{
    int current_value;

    pthread_mutex_lock(&mutex);
    current_value = flag;
    pthread_mutex_unlock(&mutex); 

    if (current_value == 1)
    {
        /* ... */
    }
    else
    {
        /* ... */
    }
}

___________

Furthermore, instead of "busy waiting" for the flag, i.e. checking the flag in loop again and again (which unnecessarily burns a whole lot of CPU cycles!), you probably want to have a look at how condition variables work:

void thread_1()
{
    pthread_mutex_lock(&mutex);
    while (flag != DESIRED_VALUE)
    {
        pthread_cond_wait(&cond, &mutex); // <-- Sleep until signaled!
    }

    /* ... */

    pthread_mutex_unlock(&mutex);
}

void thread_2()
{
    /* ... some long-runnig operation ...*/

    pthread_mutex_lock(&mutex);
    flag = DESIRED_VALUE;
    pthread_mutex_unlock(&mutex);
    pthread_cond_signal(&cond); // <-- Wake up the waiter! */
}

See also:
https://pages.cs.wisc.edu/~remzi/OSTEP/threads-cv.pdf

___________

An alternative to using locks (e.g. mutex) is using atomic operations:
* https://gcc.gnu.org/onlinedocs/gcc-4.4.3/gcc/Atomic-Builtins.html#Atomic-Builtins
* https://learn.microsoft.com/en-us/windows/win32/sync/interlocked-variable-access

[seeplus]

flag isn't an int - it's a bool flag. It's either set or reset. You probably need std::atomic_flag which is lock free.
See the atomic section of:
https://en.cppreference.com/w/cpp/thread

[kigar64551]

Atomic operations are great for something like a "shared" counter.

However, they don't solve the problem of "busy waiting" for a flag to get a certain expected value/state.

For that situation, with C++11 (or later), I'd suggest something like:

#include <mutex>
#include <condition_variable>

class Flag
{
private:
	std::mutex m_mutex;
	std::condition_variable m_cv;
	bool m_flag = false;

public:
	void set(const bool new_value);
	void wait(const bool expected_value);
};

#include "Flag.h"

void Flag::set(const bool new_value)
{
	std::unique_lock<std::mutex> lock(m_mutex);
	if (m_flag != new_value)
	{
		m_flag = new_value;
		m_cv.notify_all();
	}
}

void Flag::wait(const bool expected_value)
{
	std::unique_lock<std::mutex> lock(m_mutex);
	while (m_flag != expected_value)
	{
		m_cv.wait(lock);
	}
}

#include "Flag.h"

Flag my_flag;

int main()
{
    m_flag.wait(true);
}

#include "Flag.h"

extern Flag my_flag;

int thread_function()
{
    /* ... some long-running operation ... */

    m_flag.set(true);
}

You could do pretty much the same in pure C with pthread_mutext_t and pthread_cond_t, of course!

[Jonathan100]

Thanks guys,

First of all, I'm actually working on hardware (Xilinx), so the full code won't help you compile
Second, I learned about circular buffer. I wonder if it is suitable solution?

> After setting flag to 0, how quickly does it get set back to 1 ?

Flag is set when there in hw interrupt, then in main() it should do some procesure, and reset . There is enough time until it get to '1'. That is why I don't understand why mutex is needed?

[mbozzi]

If there's only one thread of execution involved it might be sufficient to declare the variable volatile.

You may also need to make sure the variable in the ISR is modified only atomically, so that simultaneous reads/writes, if any are actually possible, never encounter an intermediate state.

This is why signal handlers are supposed to use volatile sig_atomic_t
https://en.cppreference.com/w/cpp/utility/program/signal

Maybe try to find some example code and see how it works. I don't know anything about Xilinx.

[kigar64551]

Second, I learned about circular buffer. I wonder if it is suitable solution?

A circular buffer is commonly used to exchange data between concurrent threads. Simply put, one thread, the "producer", writes its generated data into the slots in sequential order, while the other thread, the "consumer", reads the data from the slots that have been filled, also in sequential order. Once all slots have been used up, the "producer" wraps around and starts overwriting the slots from the start.

But: The circular buffer still requires proper synchronization! You have to make sure that the "consumer" thread does not read a slot before that slot has been filled by the "producer" thread. Similarly, you have to make sure that the "producer" thread does not overwrite a slot before that slot has been fully read by the "consumer" thread. A specific slot must not be accessed by multiple threads at the same time!

A standard approach to synchronize a circular buffer is by using two semaphores: One for counting the "free" slots, and another one for counting the "used" slots. The producer thread waits until the "free" counter becomes non-zero, then decrements the "free" counter, writes its data into the next slot, and finally increments the "used" counter. Similarly, the consumer thread waits until the "used" counter becomes non-zero, then decrements the "used" counter, reads the data from the next slot, and finally increments the "free" counter.

It's important to note that, when it comes to semaphores, the "wait until counter becomes non-zero" action and the "decrement counter" action actually must happen as a single atomic operation, in order to avoid race conditions!