parallel: add support to explicitly destroy TLS keys#98
parallel: add support to explicitly destroy TLS keys#98compnerd wants to merge 1 commit intosaeta:mainfrom
Conversation
In most modern pthread implementations, the TLS key does not create heap allocations. However, this is not true for all implementations of TLS. Extend the interface to permit the destruction of the key to release resources. Also add an implementation which conforms to the POSIX threading interfaces. Extend the change a bit further to allow for the resetting of the key itself to a TLS implementation specific invalid value. Although this unconstifies the value, it will make it easier to identify invalid uses of the key (e.g. use-after-free).
| /// Makes a new key; the returned key should be used for the entire process lifetime. | ||
| static func makeKey() -> Key | ||
| /// Invalidates a previously constructed key, freeing resources | ||
| static func destroyKey(_ key: inout Key) |
There was a problem hiding this comment.
Would a better design be for the Key's destructor to handle cleanup?
There was a problem hiding this comment.
The destroyKey is different from the key destructor - it frees the TLS slot for re-use, it does not handle the cleanup of the TLS key itself, which remains the duty of the TLS destructor.
| /// Token used to index into the thread local storage. | ||
| public struct Key<Value: AnyObject> { | ||
| fileprivate let key: Underlying.Key | ||
| fileprivate var key: Underlying.Key |
There was a problem hiding this comment.
Note: if Underlying.Key is a class, then I think this change can be reverted. WDYT?
There was a problem hiding this comment.
We can revert this change even without that - I call that out in the commit message with an explanation of why I changed this. It's purely for detection purposes. If we say that the user is responsible for knowing when they have invalidated the TLS slot, this can remain a let binding. One could argue that the destroy is an explicit action - most usage will be for the lifetime of the program, in which case, you just leak the slot which is irrelevant because the application is terminating and the resources will be released as a byproduct. But, if you want to change the underlying Key to class even outside of that reasoning, I can certainly do that.
| Key(key: Underlying.makeKey()) | ||
| } | ||
|
|
||
| /// Deallocates a key for type `T`. |
| #if os(Windows) | ||
| fatalError("Unimplemented!") | ||
| #else | ||
| pthread_key_delete(key.value) |
There was a problem hiding this comment.
Let's change Key to a class, and move this into deinit. WDYT?
2 participants
In most modern pthread implementations, the TLS key does not create heap
allocations. However, this is not true for all implementations of TLS.
Extend the interface to permit the destruction of the key to release
resources.
Also add an implementation which conforms to the POSIX threading
interfaces.
Extend the change a bit further to allow for the resetting of the key
itself to a TLS implementation specific invalid value. Although this
unconstifies the value, it will make it easier to identify invalid uses
of the key (e.g. use-after-free).