sys:posix:pthread added dynamic pthread thread local storage

[BytesGalore]

added a dynamic tls implementation to pthread.c

This is an alternative approach to #827

The impact is additional 4 bytes per pthread_thread_t when not used.
If used, 12 bytes for each key and additional 12 bytes for each tls is consumed.

[Kijewski]

Please note that the overhead of malloc must not be neglected.
The code needs a lot of formatting.
The destructor is not called on the thread's death.

Compare #827, which implements the feature differently.

[BytesGalore]

Please note that the overhead of malloc must not be neglected.

absolutely. I know, this implementation is a bit naive.
I thought about to ease this problematic by pre-allocate a specific amount of memory, and reuse once allocated key storage (say for 3 keys per pthread).
But trying to predict a specific size is more or less just random and would only fit in some cases.

[BytesGalore]

ok, added destructor and cleanup calls, cleaned up formatting and rebased on current master

[Kijewski]

Huh?

[BytesGalore]

astyle

[Kijewski]

Granted, your implementation looks easier on the eyes than mine. I'll give it a proper review.

[Kijewski]

Swap these last two lines.

[BytesGalore]

reverted astyle code styling to apply it when finished

[Kijewski]

"break out iteration since unique key has been found already"

pthread_key_create() deletes the key, not only the value in the caller. For all pthreads the tls_data_t has to be free'd.

[BytesGalore]

This is one point where I'm undecided.
The opengroup states to pthread_key_delete():
...It is the responsibility of the application to free any application storage or perform any cleanup actions for data structures related to the deleted key...

and further (only destructor function related but fitting):
Thus, they cannot be used to free thread-specific data in other threads at key deletion time

[Kijewski]

The key becomes invalid, non-existent. To single thread the pointer that it stored in there is lost, essentially because an invalid key cannot be used to look up the value again.

[BytesGalore]

ok, but for pthread_key_create() it is said that:
Although the same key value may be used by different threads, the values bound to the key by pthread_setspecific() are maintained on a per-thread basis and persist for the life of the calling thread.

That's the reason why I made BytesGalore@e33c649 (tracking number of pthreads using a specific key)

[Kijewski]

E.g. Dietlibc interprets it like I do: https://github.com/ensc/dietlibc/blob/master/libpthread/pthread_key.c. pthread_key_delete() sets __thread_keys[key].used = 0, and pthread_setspecific() tests if __thread_keys[key].used != 0.

[BytesGalore]

true, tracking and keeping the keys was not the expected behaviour.
But still, I have a bad feeling to free the tls_data_t of foreign pthreads.
It's never a good idea to interfere memory management of "others"

I propose to delete the key and only this thread's data, and let the __pthread_keys_exit() function free the data of all pthreads sharing the deleted key

[LudwigKnuepfer]

@OlegHahm @Kijewski was there any outcome? Apparently this is a wanted feature and in good shape.

[OlegHahm]

We didn't look at it IIRC. I have nob objections against merging.

[OlegHahm]

Maybe @josephnoir wants to review/ack/comment?

[josephnoir]

Looked at the code, looks good and the test compiles and runs. However, I believe the test name is outdated?

[BytesGalore]

@josephnoir like naming the test without test prefix right?

[BytesGalore]

@josephnoir renamed the test, will squash when travis succeeds

[BytesGalore]

squashed

[OlegHahm]

@ALL: please do not remove your assignment without reassigning - if you don't know (or don't care) who to assign, just pick me (or whoever comes first).