Block task-local values in isolated deinit, instead of copying, following review feedback

Author: nickolas-pohilets

include/swift/ABI/TaskLocal.h

@@ -253,14 +253,6 @@ class TaskLocal {
   /// management. If @c task is nil, items will be allocated using malloc(). The
   /// same value of @c task should be passed to @c TaskLocal::Storage::destroy() .
   static void copyTo(Storage *target, AsyncTask *task);
-
-  class AdHocScope {
-    Storage *oldStorage;
-
-  public:
-    AdHocScope(Storage *storage);
-    ~AdHocScope();
-  };
 
   class WithResetValuesScope {
     bool didPush;

stdlib/public/Concurrency/Actor.cpp

@@ -1791,16 +1791,11 @@ class IsolatedDeinitJob : public Job {
 private:
   void *Object;
   DeinitWorkFunction *Work;
-  TaskLocal::Storage Local;
 public:
   IsolatedDeinitJob(JobPriority priority, void *object, DeinitWorkFunction *work)
       : Job({JobKind::IsolatedDeinit, priority}, &process), Object(object),
         Work(work)
-  {
-    TaskLocal::copyTo(&Local, nullptr);
-  }
-
-  ~IsolatedDeinitJob() { Local.destroy(nullptr); }
+  {}
 
   SWIFT_CC(swiftasync)
   static void process(Job *_job) {
@@ -1810,7 +1805,7 @@ class IsolatedDeinitJob : public Job {
   }
 
   inline void runWork() {
-    TaskLocal::AdHocScope taskLocalScope(&Local);
+    TaskLocal::WithResetValuesScope taskLocalResetScope;
     Work(Object);
   }
 
@@ -1831,7 +1826,8 @@ static void swift_task_deinitOnExecutorImpl(void *object,
   // Note that swift_task_isCurrentExecutor() returns true for @MainActor
   // when running on the main thread without any executor
   if (swift_task_isCurrentExecutor(newExecutor)) {
-    return work(object); // 'return' forces tail call
+    TaskLocal::WithResetValuesScope taskLocalResetScope;
+    return work(object);
   }
 
   // Optimize deallocation of the default actors
@@ -1853,7 +1849,10 @@ static void swift_task_deinitOnExecutorImpl(void *object,
       trackingInfo.enterAndShadow(newExecutor);
 
       // Run the work.
-      work(object);
+      {
+        TaskLocal::WithResetValuesScope taskLocalResetScope;
+        work(object);
+      }
 
       // `work` is a synchronous function, it cannot call swift_task_switch()
       // If it calls any synchronous API that may change executor inside tracking info,

stdlib/public/Concurrency/TaskLocal.cpp

@@ -425,17 +425,6 @@ void TaskLocal::Storage::copyTo(TaskLocal::Storage *target, AsyncTask *task) {
   }
 }
 
-TaskLocal::AdHocScope::AdHocScope(Storage *storage) {
-  assert(swift_task_getCurrent() == nullptr &&
-         "Cannot use ad-hoc scope with a task");
-  oldStorage = FallbackTaskLocalStorage::get();
-  FallbackTaskLocalStorage::set(storage);
-}
-
-TaskLocal::AdHocScope::~AdHocScope() {
-  FallbackTaskLocalStorage::set(oldStorage);
-}
-
 TaskLocal::WithResetValuesScope::WithResetValuesScope() {
     didPush = swift_task_localStopPush();
 }

test/Concurrency/Runtime/async_task_locals_isolated_deinit.swift

@@ -75,7 +75,7 @@ if #available(SwiftStdlib 5.1, *) {
     Task {
       await TL.$number.withValue(42) {
         await AnotherActor.shared.performTesting {
-          _ = ClassWithIsolatedDeinit(expectedNumber: 42, group: group)
+          _ = ClassWithIsolatedDeinit(expectedNumber: 0, group: group)
         }
       }
     }
@@ -88,10 +88,10 @@ if #available(SwiftStdlib 5.1, *) {
     group.enter()
     Task {
       TL.$number.withValue(37) {
-        _ = ActorWithIsolatedDeinit(expectedNumber: 37, group: group)
+        _ = ActorWithIsolatedDeinit(expectedNumber: 0, group: group)
       }
       TL.$number.withValue(99) {
-        _ = ClassWithIsolatedDeinit(expectedNumber: 99, group: group)
+        _ = ClassWithIsolatedDeinit(expectedNumber: 0, group: group)
       }
     }
     group.wait()