Avoid dropping TLS Key on sgx

[raoulstrackx]

#102655 reenabled dropping thread local Key on every platform (library/std/src/sys_common/thread_local_key.rs). That's causing problems at least for sgx.

cc: @jethrogb @ChrisDenton

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

[ChrisDenton]

I apologise, I assumed from a comment in the code that only Windows would be affected but I really should have checked.

I wonder if a more principled approach would be to add a function to sys/{os}/thread_local_key? Something like:

// library/std/src/sys/sgx/thread_local_key.rs
#[inline(always)]
pub fn destroy_on_drop() -> bool {
    false
}

I.e. something similar to the now removed requires_synchronized_create.

[joboet]

Sorry about this, I didn't check for this in the original PR. Also, how about clearing the destructor on key destruction. That would also solve the problem, if I understand correctly. Nevermind, that seems fine.

Could you share details on the problem?

[jethrogb]

We're still investigating the issue, but the symptom is that the thread::local::dynamic_tests::hashmap test segfaults.

[jethrogb]

Can we please merge this to get std tests passing again and then work on a permanent solution?

[joboet]

Does this really fix the test? Because that test does not even use thread_local_key::Key but rather (through some abstractions) thread_local_key::StaticKey, which did not change on SGX.

[raoulstrackx]

Does this really fix the test? Because that test does not even use thread_local_key::Key but rather (through some abstractions) thread_local_key::StaticKey, which did not change on SGX.

Yes it does. I'm still investigating why destroying the key leads to issues.

[raoulstrackx]

Found the main culprit! Data wasn't always kept in sync that caused StaticKey to return incorrect pointer values when the same key was used previously.

[ChrisDenton]

LGTM

@bors r+ rollup

[bors]

📌 Commit c46185b has been approved by ChrisDenton

It is now in the queue for this repository.