[Windows] Fix thread local storage cleanup

[lxbndr]

CoreFoundation thread local storage on Windows is based on FLS API. FLS provides automatic cleanup of storage, and it is already got used. Old manual cleanup is not needed anymore.

We have observed often crashes in thread cleanup routines of CRT. Despite I didn't manage to extract the issue out of live project code, I suppose that mixing TLS and FLS in cleanup code could eventually corrupt the storage. Replacing TlsGetValue with FlsGetValue brings desired stability, but, as the code is obsolete, we could just remove it.

[lxbndr]

CC @compnerd

[compnerd]

Thanks for finding and fixing this. Yes, the CoreFoundation implementation switched to Fls* to get a destructor-enabled thread local implementation. This should have been changed to FlsGetValue when that change was made.

[compnerd]

@swift-ci please test

[lxbndr]

@compnerd BTW do we still need __CFFinalizeWindowsThreadData? It is called from DllMain on DLL_THREAD_DETACH event. That was needed for TLS-based implementation, but FLS provides destructor mechanics. I see that __CFTSDFinalize is actually called twice. First time is from __CFFinalizeWindowsThreadData. I guess second call is from FLS itself.

I could make sure it is actual behavior and remove __CFFinalizeWindowsThreadData in this patch, instead of fixing it. WDYT?

[compnerd]

Hmm, if the DLL_THREAD_DETACH is only doing this to handle the cleanup on the unload, we should remove it as the FLS will be sufficient for the cleanup.

[lxbndr]

I will run extended tests to make sure everything is ok without DLL_THREAD_DETACH cleanup. Converting this to draft for now.

[lxbndr]

Seems like everything is ok and we could just remove old finalization instead of fixing it. Did not run into any new issues for last days.

[compnerd]

@swift-ci please test

[compnerd]

CC: @millenomi any objections to the change to CF here?

[spevans]

Is this bugfix worth backporting to 5.3?