sys/posix/pthread/pthread.c: fix thread count limiting in pthread_create()

[JulianHolzwarth]

Contribution description

While looking at posix threads I found a bug. When creating more threads than possible pthread_create does not check correctly for errors.

First change in insert because of :

 kernel_pid_t pthread_pid = insert(pt);
    if (pthread_pid == KERNEL_PID_UNDEF) {

second change because thread_create returns -EOVERFLOW not KERNEL_PID_UNDEF when returning an error (we dont need to check the other error because the priority is main thread prority)

Testing procedure

Try to create more than MAXTHREADS pthreads and join on every successfully created thread.
Before the fix it
Should I implement a test for it?

Issues/PRs references

[JulianHolzwarth]

@PeterKietzmann Could you review this pr please? It looks like you are a maintainer for the POSIX Layer.

[JulianHolzwarth]

@smlng Could you look at this? I was told you could be the right person.

[smlng]

regarding testing: maybe copy from tests/thread_flood i.e. provide a pthread variant of that in the same style?

[smlng]

in error case thread_create can return -EAGAIN, -EOVERFLOW or KERNEL_PID_UNDEF, hence I would suggest to check for <= KERNEL_PID_UNDEF?

[JulianHolzwarth]

I dont think it can return KERNEL_PID_UNDEF because it has 3 returns
Code from core/thread.c

if (pid == KERNEL_PID_UNDEF) {
        DEBUG("thread_create(): too many threads!\n");

        irq_restore(state);

        return -EOVERFLOW;
    }

return pid; cannot return KERNEL_PID_UNDEF because of previous code if (pid == KERNEL_PID_UNDEF) { ... return -EOVERFLOW

and this return that only returns when the priority is wrong. But the priority is always THREAD_PRIORITY_MAIN so this should never return -EINVAL

if (priority >= SCHED_PRIO_LEVELS) {
        return -EINVAL;
    }

That is why I only check for -EOVERFLOW
But the check <= KERNEL_PID_UNDEF could be easier to read.
Would you still suggest the change?

[smlng]

mhm, maybe check for < KERNEL_PID_UNDEF then, bc I wouldn't rely on the priority to work either - e.g. someone might redefine THREAD_PRIORITY_MAIN making it invalid.

However, you're right it can never return KERNEL_PID_UNDEF.

[JulianHolzwarth]

I will use the function pid_is_valid that should be the right way to do it.

[smlng]

nice catch!

[JulianHolzwarth]

Implementing a test is a good idea. Will do.
Should it be in the same pr? @smlng

[smlng]

Implementing a test is a good idea. Will do.
Should it be in the same pr? @smlng

sure, but extra commit

[JulianHolzwarth]

I added a small test. I can add more comments if needed.

[JulianHolzwarth]

removed unnecessary includes

[smlng]

please fix CI issue and squash directly

[JulianHolzwarth]

done

[smlng]

tested ACK

[smlng]

looks like you need to put the nucleo-f031k6 board into the list of insufficient memory boards.

[JulianHolzwarth]

Can I rebase? ( Fixup that added BOARD_INSUFFICIENT_MEMORY := nucleo-f031k6 )

[smlng]

rebase and (most importantly) squash, yes please

[JulianHolzwarth]

done