back to println tests

Author: ktoso

stdlib/public/Concurrency/TaskGroup.cpp

@@ -64,7 +64,7 @@
 
 using namespace swift;
 
-#if 1
+#if 0
 #define SWIFT_TASK_GROUP_DEBUG_LOG(group, fmt, ...)                     \
 fprintf(stderr, "[%#lx] [%s:%d][group(%p%s)] (%s) " fmt "\n",           \
       (unsigned long)Thread::current().platformThreadId(),              \
@@ -390,7 +390,11 @@ class TaskGroupBase : public TaskGroupTaskStatusRecord {
   virtual void enqueueCompletedTask(AsyncTask *completedTask, bool hadErrorResult) = 0;
 
   /// Resume waiting task with result from `completedTask`
-  void resumeWaitingTask(AsyncTask *completedTask, TaskGroupStatus &assumed, bool hadErrorResult, bool alreadyDecremented = false);
+  void resumeWaitingTask(AsyncTask *completedTask,
+                         TaskGroupStatus &assumed,
+                         bool hadErrorResult,
+                         bool alreadyDecremented = false,
+                         bool taskWasRetained = false);
 
   // ==== Status manipulation -------------------------------------------------
 
@@ -827,7 +831,9 @@ class DiscardingTaskGroup: public TaskGroupBase {
 
 private:
   /// Resume waiting task with specified error
-  void resumeWaitingTaskWithError(SwiftError *error, TaskGroupStatus &assumed, bool alreadyDecremented);
+  void resumeWaitingTaskWithError(SwiftError *error,
+                                  TaskGroupStatus &assumed,
+                                  bool alreadyDecremented);
 };
 
 } // end anonymous namespace
@@ -1040,7 +1046,7 @@ static void fillGroupNextResult(TaskFutureWaitAsyncContext *context,
 
   case PollStatus::Error: {
     auto error = reinterpret_cast<SwiftError *>(result.storage);
-    fillGroupNextErrorResult(context, error); // FIXME: this specifically retains the error, but likely should not!??!!?
+    fillGroupNextErrorResult(context, error);
     return;
   }
 
@@ -1240,11 +1246,16 @@ void DiscardingTaskGroup::offer(AsyncTask *completedTask, AsyncContext *context)
         switch (readyErrorItem.getStatus()) {
           case ReadyStatus::RawError:
             SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, complete, resume with raw error:%p", readyErrorItem.getRawError(this));
-            resumeWaitingTaskWithError(readyErrorItem.getRawError(this), assumed, alreadyDecrementedStatus);
+            resumeWaitingTaskWithError(readyErrorItem.getRawError(this), assumed,
+                                       alreadyDecrementedStatus);
             break;
           case ReadyStatus::Error:
             SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, complete, resume with errorItem.task:%p", readyErrorItem.getTask());
-            resumeWaitingTask(readyErrorItem.getTask(), assumed, /*hadErrorResult=*/true, alreadyDecrementedStatus);
+            SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, complete, expect that it was extra retained %p", readyErrorItem.getTask());
+            resumeWaitingTask(readyErrorItem.getTask(), assumed,
+                              /*hadErrorResult=*/true,
+                              alreadyDecrementedStatus,
+                              /*taskWasRetained=*/true);
             break;
           default:
             swift_Concurrency_fatalError(0,
@@ -1283,7 +1294,10 @@ void DiscardingTaskGroup::offer(AsyncTask *completedTask, AsyncContext *context)
           resumeWaitingTaskWithError(readyErrorItem.getRawError(this), assumed, alreadyDecrementedStatus);
           break;
         case ReadyStatus::Error:
-          resumeWaitingTask(readyErrorItem.getTask(), assumed, /*hadErrorResult=*/true, alreadyDecrementedStatus);
+          resumeWaitingTask(readyErrorItem.getTask(), assumed,
+                            /*hadErrorResult=*/true,
+                            alreadyDecrementedStatus,
+                            /*taskWasRetained=*/true);
           break;
         default:
           swift_Concurrency_fatalError(0,
@@ -1293,14 +1307,6 @@ void DiscardingTaskGroup::offer(AsyncTask *completedTask, AsyncContext *context)
       // This is the last task, we have a waiting task and there was no error stored previously;
       // We must resume the waiting task with a success, so let us return here.
       resumeWaitingTask(completedTask, assumed, /*hadErrorResult=*/false, alreadyDecrementedStatus);
-
-//      // TODO: since the DiscardingTaskGroup ended up written as `-> T` in order to use the same pointer auth as the
-//      //       usual task closures; we end up retaining the value when it is returned. As this is a discarding group
-//      //       we actually can and should release this value.
-//      //       Is there a way we could avoid the retain made on the returned value entirely?
-//      if (completedTask->futureFragment()->getResultType()->isClassObject()) {
-//        swift_release(reinterpret_cast<HeapObject *>(completedTask->futureFragment()->getStoragePtr()));
-//      }
     }
   } else {
     // it wasn't the last pending task, and there is no-one to resume;
@@ -1317,7 +1323,8 @@ void TaskGroupBase::resumeWaitingTask(
     AsyncTask *completedTask,
     TaskGroupStatus &assumed,
     bool hadErrorResult,
-    bool alreadyDecremented) {
+    bool alreadyDecremented,
+    bool taskWasRetained) {
   auto waitingTask = waitQueue.load(std::memory_order_acquire);
   assert(waitingTask && "waitingTask must not be null when attempting to resume it");
   assert(assumed.hasWaitingTask());
@@ -1384,14 +1391,14 @@ void TaskGroupBase::resumeWaitingTask(
         auto before = completedTask;
         _swift_taskGroup_detachChild(asAbstract(this), completedTask);
         SWIFT_TASK_GROUP_DEBUG_LOG(this, "completedTask %p; AFTER DETACH (count:%d)", completedTask, swift_retainCount(completedTask));
-        if (hadErrorResult) {
+        if (isDiscardingResults() && hadErrorResult && taskWasRetained) {
           SWIFT_TASK_GROUP_DEBUG_LOG(this, "BEFORE RELEASE error task=%p (count:%d)\n",
                   completedTask,
                   swift_retainCount(completedTask));
           // We only used the task to keep the error in the future fragment around
           // so now that we emitted the error and detached the task, we are free to release the task immediately.
-          auto error = reinterpret_cast<SwiftError *>(result.storage);
-          swift_release(completedTask); // we need to do this if the error is a class
+          auto error = completedTask->futureFragment()->getError();
+          swift_release(completedTask);
         }
 
         _swift_tsan_acquire(static_cast<Job *>(waitingTask));

stdlib/public/runtime/HeapObject.cpp

@@ -525,15 +525,13 @@ void (*SWIFT_RT_DECLARE_ENTRY _swift_release)(HeapObject *object) =
     _swift_release_;
 
 void swift::swift_nonatomic_release(HeapObject *object) {
-  fprintf(stderr, "[%s:%d](%s) swift_nonatomic_release %p\n", __FILE_NAME__, __LINE__, __FUNCTION__, object);
   SWIFT_RT_TRACK_INVOCATION(object, swift_nonatomic_release);
   if (isValidPointerForNativeRetain(object))
     object->refCounts.decrementAndMaybeDeinitNonAtomic(1);
 }
 
 SWIFT_ALWAYS_INLINE
 static void _swift_release_n_(HeapObject *object, uint32_t n) {
-fprintf(stderr, "[%s:%d](%s) _swift_release_n_ %p (n=%d)\n", __FILE_NAME__, __LINE__, __FUNCTION__, object, n);
   SWIFT_RT_TRACK_INVOCATION(object, swift_release_n);
   if (isValidPointerForNativeRetain(object))
     object->refCounts.decrementAndMaybeDeinit(n);
@@ -797,8 +795,6 @@ void swift::swift_unownedCheck(HeapObject *object) {
 }
 
 void _swift_release_dealloc(HeapObject *object) {
-  fprintf(stderr, "[%s:%d](%s) _swift_release_dealloc %p (count before: %d)\n", __FILE_NAME__, __LINE__, __FUNCTION__, object,
-          swift_retainCount(object));
   asFullMetadata(object->metadata)->destroy(object);
 }
 

test/Concurrency/Runtime/async_taskgroup_discarding.swift

@@ -1,4 +1,5 @@
-// RUN: %target-run-simple-leaks-swift( -Xfrontend -disable-availability-checking -parse-as-library)
+// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s
+// TODO: move to target-run-simple-leaks-swift once CI is using at least Xcode 14.3
 
 // REQUIRES: concurrency
 // REQUIRES: executable_test
@@ -11,31 +12,30 @@
 // UNSUPPORTED: OS=windows-msvc
 
 import _Concurrency
-import Darwin
 
-final class FIRST_PAYLOAD {}
-final class SECOND_PAYLOAD {}
+final class PayloadFirst {}
+final class PayloadSecond {}
 
-final class FIRST_ERROR: Error {
-  let first: FIRST_PAYLOAD
+final class ErrorFirst: Error {
+  let first: PayloadFirst
 
   init(file: String = #fileID, line: UInt = #line) {
     first = .init()
   }
   deinit {
-    fputs("\(Self.self) deinit\n", stderr)
+    print("deinit \(self)")
   }
 }
 
-final class SECOND_ERROR: Error {
-  let second: SECOND_PAYLOAD
+final class ErrorSecond: Error {
+  let second: PayloadSecond
 
   init(file: String = #fileID, line: UInt = #line) {
     second = .init()
   }
 
   deinit {
-    fputs("\(Self.self) deinit\n", stderr)
+    print("deinit \(self)")
   }
 }
 
@@ -55,20 +55,20 @@ func shouldStartWith(_ lhs: Any, _ rhs: Any) {
           1
         }
         group.addTask {
-          throw FIRST_ERROR()
+          throw ErrorFirst()
         }
 
-//        try? await Task.sleep(for: .seconds(1))
-
         group.addTask {
-          throw SECOND_ERROR()
+          throw ErrorSecond()
         }
 
         return 12
       }
       fatalError("expected error to be re-thrown, got: \(got)")
     } catch {
-      // shouldStartWith(error, "main.Boom")
+       shouldStartWith(error, "main.Error")
     }
+    // CHECK: deinit main.Error
+    // CHECK: deinit main.Error
   }
 }

test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift

@@ -1,7 +1,5 @@
-// RUN: %target-run-simple-leaks-swift( -Xfrontend -disable-availability-checking -parse-as-library)
-
-// This test uses `leaks` which is only available on apple platforms; limit it to macOS:
-// REQUIRES: OS=macosx
+// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s --dump-input=always
+// TODO: move to target-run-simple-leaks-swift once CI is using at least Xcode 14.3
 
 // REQUIRES: concurrency
 // REQUIRES: executable_test
@@ -15,38 +13,45 @@
 
 import _Concurrency
 
-struct Boom: Error {
+final class PrintDeinit {
   let id: String
+  init(id: String) {
+    self.id = id
+  }
 
-  init(file: String = #fileID, line: UInt = #line) {
-    self.id = "\(file):\(line)"
+  deinit {
+    print("deinit, id: \(id)")
   }
+}
+
+struct Boom: Error {
+  let printDeinit: PrintDeinit
 
   init(id: String) {
-    self.id = id
+    self.printDeinit = PrintDeinit(id: id)
   }
 }
 
 final class BoomClass: Error {
   let id: String
 
-  init(file: String = #fileID, line: UInt = #line) {
-    self.id = "\(file):\(line)"
-  }
-
   init(id: String) {
     self.id = id
   }
+
+  deinit {
+    print("deinit, id: \(id)")
+  }
 }
 
 final class SomeClass: @unchecked Sendable {
-//struct SomeClass: @unchecked Sendable {
-  let number: Int
-  init() {
-    self.number = 0
+  let id: String
+  init(id: String) {
+    self.id = id
   }
-  init(number: Int) {
-    self.number = number
+
+  deinit {
+    print("deinit, id: \(id)")
   }
 }
 
@@ -56,21 +61,31 @@ final class SomeClass: @unchecked Sendable {
   static func main() async {
     _ = try? await withThrowingDiscardingTaskGroup() { group in
       group.addTask {
-        throw Boom()
+        throw Boom(id: "race-boom-class")
       }
       group.addTask {
-        SomeClass() // will be discarded
+        SomeClass(id: "race-boom-class") // will be discarded
       }
+      // since values may deinit in any order, we just assert their count basically
+      // CHECK: deinit, id: race-boom-class
+      // CHECK: deinit, id: race-boom-class
 
       return 12
     }
 
     // many ok
     _ = try? await withThrowingDiscardingTaskGroup() { group in
-      for i in 0..<10 {
+      for i in 0..<6 {
         group.addTask {
-          SomeClass(number: i) // will be discarded
+          SomeClass(id: "many-ok") // will be discarded
         }
+        // since values may deinit in any order, we just assert their count basically
+        // CHECK: deinit, id: many-ok
+        // CHECK: deinit, id: many-ok
+        // CHECK: deinit, id: many-ok
+        // CHECK: deinit, id: many-ok
+        // CHECK: deinit, id: many-ok
+        // CHECK: deinit, id: many-ok
       }
 
       return 12
@@ -79,12 +94,20 @@ final class SomeClass: @unchecked Sendable {
     // many throws
     do {
       let value = try await withThrowingDiscardingTaskGroup() { group in
-        for i in 0..<10 {
+        for i in 0..<6 {
           group.addTask {
-            throw BoomClass() // will be rethrown
+            throw BoomClass(id: "many-error") // will be rethrown
           }
         }
 
+        // since values may deinit in any order, we just assert their count basically
+        // CHECK: deinit, id: many-error
+        // CHECK: deinit, id: many-error
+        // CHECK: deinit, id: many-error
+        // CHECK: deinit, id: many-error
+        // CHECK: deinit, id: many-error
+        // CHECK: deinit, id: many-error
+
         12 // must be ignored
       }
       preconditionFailure("Should throw")
@@ -95,27 +118,34 @@ final class SomeClass: @unchecked Sendable {
     // many errors, many values
     _ = try? await withThrowingDiscardingTaskGroup() { group in
       group.addTask {
-        SomeClass() // will be discarded
+        SomeClass(id: "mixed-ok") // will be discarded
       }
       group.addTask {
-        SomeClass() // will be discarded
+        SomeClass(id: "mixed-ok") // will be discarded
       }
       group.addTask {
-        SomeClass() // will be discarded
+        SomeClass(id: "mixed-ok") // will be discarded
       }
       group.addTask {
-        throw Boom()
+        throw Boom(id: "mixed-error")
       }
       group.addTask {
-        throw Boom()
+        throw Boom(id: "mixed-error")
       }
       group.addTask {
-        throw Boom()
-      }
-      group.addTask {
-        throw Boom()
+        throw Boom(id: "mixed-error")
       }
 
+      // since values may deinit in any order, we just assert their count basically
+      // three ok's
+      // CHECK: deinit, id: mixed
+      // CHECK: deinit, id: mixed
+      // CHECK: deinit, id: mixed
+      // three errors
+      // CHECK: deinit, id: mixed
+      // CHECK: deinit, id: mixed
+      // CHECK: deinit, id: mixed
+
       return 12
     }
   }