Mbed-os: I can not resume thread with rtos

[tichise]

Description

I can not resume thread with rtos. I write the program as follows, but when I execute the second callback, the start method returns osErrorParameter with osStatus.
Please tell me how to fix it.

#include "mbed.h"

Thread thread;
DigitalOut led1(LED1);
volatile bool running = true;

void printOSStatus(osStatus status)
{
    if (status == osOK)
    {
        printf("osOK\n");
    }
    else if (status == osErrorValue)
    {
        printf("osErrorValue\n");
    }
    else if (status == osErrorResource)
    {
        printf("osErrorResource\n");
    }
    else if (status == osErrorTimeoutResource)
    {
        printf("osErrorTimeoutResource\n");
    }
    else if (status == osErrorNoMemory)
    {
        printf("osErrorNoMemory\n");
    }
    else if (status == osErrorISR)
    {
        printf("osErrorISR\n");
    }
    else if (status == osErrorISRRecursive)
    {
        printf("osErrorISRRecrusive\n");
    }
    else if (status == osErrorPriority)
    {
        printf("osErrorPriority\n");
    }
    else if (status == osErrorParameter)
    {
        printf("osErrorParameter\n");
    }
    else if (status == osErrorOS)
    {
        printf("osErrorOS\n");
    }
    else if (status == osEventSignal)
    {
        printf("osEventSignal\n");
    }
    else if (status == osEventMessage)
    {
        printf("osEventMessage\n");
    }
    else if (status == osEventMail)
    {
        printf("osEventMail\n");
    }
    else if (status == osEventTimeout)
    {
        printf("osEventTimeout\n");
    }
    else
    {
        printf("else\n");
    }
}

void blink(DigitalOut *led) {
    printf("blink start\n");

    while (running) {
        *led = !*led;

        printf("blink while\n");

        wait(1);
    }
}

int main() {
    printf("main\n");

    osStatus status = thread.start(callback(blink, &led1));
    printOSStatus(status);

    wait(3);

    running = false;
    thread.join();
    printf("running = false\n");
    wait(3);

    status = thread.start(callback(blink, &led1));
    printOSStatus(status);

    printf("running = start\n");

    wait(1);

    running = false;
    thread.join();
    printf("running = false\n");
}

Issue request type

[x] Question
[ ] Enhancement
[ ] Bug

[kjbracey-arm]

I don't believe the API permits you to reuse a Thread object - to do this with blink as it stands you will need a second Thread object.

If you're stopping it to reclaim dynamic resources, like the stack, then it makes sense to delete your Thread object to ensure everything is released.

If you aren't wanting to release dynamic resources (it may not be desirable to, because of potential memory fragmentation - you may want to keep all memory claimed), then it's simpler to not actually kill the thread - just make it pause itself.

You could make it like this into the loop:

 bool run;
 bool pause;
 bool paused;
 Mutex mutex;
 ConditionVariable signal_blink(mutex);
 ConditionVariable signal_main(mutex);

 mutex.lock();
 while (run) {
     while (pause) {
         paused = true;
         signal_main.notify_one();
         signal_blink.wait();
     }
     paused = false;
     mutex.unlock();
     blink
     wait 1
     mutex.lock();
}
mutex.unlock();

---main

wait(3);

printf("pause = true\n");
mutex.lock();
pause = true;
while (!paused) {
    signal_main.wait();
}
mutex.unlock();

printf("paused = true\n");
wait(3);

printOSStatus(status);

mutex.lock();
pause = false;
signal_blink.notify_one();
mutex.unlock();
print("pause = false\n", pause);

wait(1);

print("paused = %d\n", paused);

printf("run = false\n");
mutex.lock();
run = false;
mutex.unlock();
thread.join();
printf("thread finished\n");

This version protects all the multi-thread variables with a Mutex, because that's required for the ConditionVariable synchronisation on pause and paused. (Your original version didn't need a Mutex, but should have used atomic functions rather than volatile for the thread-shared running.)

There are two condition variables - one used to notify blinky when pause becomes false, so it knows to restart, and one used to notify main when paused becomes true, so it knows the thread has paused.

I generally find condition variables the easiest thing to reason about when attempting to synchronise threads like this; one thread needs to notify another, and the other thread needs to wait for something.

Read up on them if you're not familiar.

[tichise]

@ kjbracey-arm Thank you for your reply.

What I want to do this time is to stop and re-execute thread. If you can destroy the thread and run the thread again, I want to know how.

I want to do the following things. Am I wrong in using the terminate function?

#include "mbed.h"

Thread thread;

void printOSStatus(osStatus status)
{
    if (status == osOK)
    {
        printf("osOK\n");
    }
    else if (status == osErrorValue)
    {
        printf("osErrorValue\n");
    }
    else if (status == osErrorResource)
    {
        printf("osErrorResource\n");
    }
    else if (status == osErrorTimeoutResource)
    {
        printf("osErrorTimeoutResource\n");
    }
    else if (status == osErrorNoMemory)
    {
        printf("osErrorNoMemory\n");
    }
    else if (status == osErrorISR)
    {
        printf("osErrorISR\n");
    }
    else if (status == osErrorISRRecursive)
    {
        printf("osErrorISRRecrusive\n");
    }
    else if (status == osErrorPriority)
    {
        printf("osErrorPriority\n");
    }
    else if (status == osErrorParameter)
    {
        printf("osErrorParameter\n");
    }
    else if (status == osErrorOS)
    {
        printf("osErrorOS\n");
    }
    else if (status == osEventSignal)
    {
        printf("osEventSignal\n");
    }
    else if (status == osEventMessage)
    {
        printf("osEventMessage\n");
    }
    else if (status == osEventMail)
    {
        printf("osEventMail\n");
    }
    else if (status == osEventTimeout)
    {
        printf("osEventTimeout\n");
    }
    else
    {
        printf("else\n");
    }
}

void test()
{
    printf("test start\n");

    while (true)
    {
        printf("while\n");

        wait(1);
    }
}

int main()
{
    printf("main\n");

    osStatus status;

    status = thread.start(callback(test));
    printOSStatus(status);

    thread.terminate();
    wait(3);

    status = thread.start(callback(test));
    printOSStatus(status); // occur osErrorParameter
}

[kjbracey-arm]

You can't re-use the Thread object for a second run. You'll need to make a new one for each start.

You could do that on the heap:

 Thread *thread;
 main() {
      thread = new Thread;
      thread->start();
      running = false;
      thread->join();
      delete thread;
      // repeat
 }

Or you could do it on the stack:

   main() {
       for (;;) {
            Thread thread;
            thread.start();
            running = false;
            thread.join();
       }
  }

Doesn't matter much the difference between those two - in both cases it's the actual stack for the thread that's a big allocation - if you're not passing it in as a static array it will be alloced and released each time.

The Thread object itself is fairly small (<100 bytes).

[kjbracey-arm]

Am I wrong in using the terminate function?

terminate is a bit scary. It's always better to ask the thread to kill itself, then join.

Regardless, the Thread object will not permit a second start. This is a deliberate design decision - the Thread object is associated with exactly one thread. A new thread needs a new object.

You say you're "resuming" the thread, but you're not really. It's a completely new thread that just happens to be doing the same thing. It doesn't get to re-use an already-used Thread object.

(Similar thing happen in other protocols - you can't portably reuse a socket for a second TCP connection, for example. A new connection needs a new socket).

[tichise]

@kjbracey-arm I understood a little. I write and try. Please give me some advice if you get lost again.

[pan-]

If you really want to reuse memory, you can destroy a local Thread object and reinitialise it with placement new. If you want to avoid memory fragmentation; you need to provide the memory chance that is going to be used as the stack:

uint8_t thread_stack[STACK_SIZE];

main() {
    Thread thread(osPriorityNormal, sizeof(thread_stack), thread_stack);
    while(true) {
        thread.start();
        running = false;
        thread.join();
        thread.~Thread();        // destroy the local thread object
        // construct a new thread at emplacement &thread
        new (&thread) Thread(osPriorityNormal, sizeof(thread_stack), thread_stack)
    }
  }

[kjbracey-arm]

@pan- Good general point on the stack, but I don't think that's a good example for the placement new. If Thread is automatic, you can hopefully do it more neatly by letting the old Thread go out of scope, like my example above. (And if that's not possible, it must be a pretty weird-looking function). Would make sense if you really want the Thread object to be static.

I've been thinking we probably should have something to support static memory reuse/overlays like that. Like SingletonPtr, but multiple users in a union.

Something like std::any's API, but with available possible types declared up-front to define built-in storage size? Getting a bit off-topic though.

[pan-]

Sorry for the bad examples; for sure in a loop it is better to use RAII. But placement new might be useful to reuse a thread object if there's really pressure on memory. In that case, I believe the best would be a move constructor.

Something like std::any's API, but with available possible types declared up-front to define built-in storage size? Getting a bit off-topic though.

std::variant sounds like what you're describing. any is basically glorified void* to an object.

[kjbracey-arm]

std::variant sounds like what you're describing.

Aha, yes, that's what I want! If we have that, then it looks like:

uint8_t thread_stack[STACK_SIZE];
std::variant<std::monostate, rtos::Thread> store;

main() {
    while(true) {
        Thread &thread = store.emplace<Thread>(osPriorityNormal, sizeof(thread_stack), thread_stack);
        thread.start();
        running = false;
        thread.join();
        // If you wanted to manually destroy, but not needed in this example
        // store.emplace<std::monostate>();
}

[0xc0170]

@tichise if you have any more questions, please follow up on forum.mbed.com . I'll close this as resolved