Mbed-os: Thread::gettid() does not return the ID of the object it is invoked on

[marcuschangarm]

Thread* t1 = new Thread(...);

// get new thread ID
osThreadId id1 = t1->gettid();

// get current thread ID
osThreadId id2 = Thread::gettid();

id1 should be the id of the new thread, t1. Instead id1 == id2.

[0xc0170]

The problem here is we can't overload gettid. We migth need to introduce a new method.

We could have gittid() actually return the instance's thread id, and getCurrentThreadId to do what gettid does now, but that would be breaking change. Let's just introduce getter method for threadid member

[ciarmcom]

ARM Internal Ref: IOTMORF-175

[0xc0170]

cc @LiyouZhou

[daniel-cesarini-tridonic-com]

@0xc0170 @sg- : I am also encountering same issue. Are there plans to change this in near future?
Actually it would be enough to have a method to return _tid variable value from the Thread class, or am I wrong?
Having the gettid() returning the runningThreadId and not the threadOnWhichGettidIsCalled is counter-intuitive.

[pan-]

In C++ 11, APIs applying to the current thread are grouped within the namespace this_thread to avoid confusion between static and member functions. Other functions such as yield or wait or delay_until might also benefits from such construct.

@sg would that be the right approach to tackle that issue on mbed os ?

[0xc0170]

In C++ 11, APIs applying to the current thread are grouped within the namespace this_thread to avoid confusion between static and member functions. Other functions such as yield or wait or delay_until might also benefits from such construct.

Good point 👍 Link for the cpp thread reference: http://en.cppreference.com/w/cpp/thread (this_thread) namespace.

Introducing a new namespace would means that current method gittid would go into this new namespace, and gittid method (outside of this_thread) would return current running thread id. Sounds fine to me (better than above proposal just a new getter for current thread id).

[kjbracey-arm]

Agree that conceptually separating "thread" and "this thread" would make sense. Current naming derives from CMSIS-RTOS, where it's osThreadGetId(void) - but in C the lack of arguments makes its meaning more obvious than Thread::gettid() being quietly static.

Looking at the static Thread functions, they could be reassigned thus:

• signal_clr, signal_wait, wait, yield, get_id ->ThisThread
• attach_idle_hook, attach_terminate_hook -> Kernel

and I suggest Thread::get_id() for the new functionality, sidestepping the existing gettid name and aligning with C++11.

(See #5419 for my under-review PR that adds Kernel)

[MarceloSalazar]

@daniel-cesarini-tridonic-com FYI https://github.com/ARMmbed/mbed-os/pull/5419 is merged.
Could you please comment whether this issue is valid or can be closed?

[daniel-cesarini-tridonic-com]

@MarceloSalazar : I do not see connection between #5419 and this issue. In #5419 there is no mention on getTid, from what I see.

[kjbracey-arm]

The issue does remain - I have some draft work following up this issue - making the change I suggested above. Not reached PR stage yet though - got other stuff I'm supposed to be doing.

https://github.com/kjbracey-arm/mbed-os/commits/thisthread

[ciarmcom]

ARM Internal Ref: MBOTRIAGE-810

[bulislaw]

Any updates?

[kjbracey-arm]

Sorry, no update. Don't think I've done anything since that draft branch. Is it time to chase this up?

I don't actually recall what was missing from that draft - whether there was anything other than documentation or tests. I think I was maybe a little unhappy with the awkwardness of the flags_wait APIs.