From 019a84a95f5cdf3736f92cde5087a5563316e262 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Mon, 8 May 2023 17:22:56 +0900 Subject: [PATCH 1/8] [lit] introduce target-run-simple-leaks-swift to run with `leaks` [lit] cleanup lit leaks target --- test/lit.cfg | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/test/lit.cfg b/test/lit.cfg index 7fa6d505d00cb..571b49b391b01 100644 --- a/test/lit.cfg +++ b/test/lit.cfg @@ -1008,7 +1008,7 @@ else: swift_reflection_test_name = 'swift-reflection-test' + variant_suffix def use_interpreter_for_simple_runs(): - def make_simple_target_run(gyb=False, stdlib=False, parameterized=False): + def make_simple_target_run(gyb=False, stdlib=False, parameterized=False, leaks=False): result = '' if gyb: result += ('%empty-directory(%t) && ' @@ -1036,6 +1036,7 @@ def use_interpreter_for_simple_runs(): config.target_run_stdlib_swift = make_simple_target_run(stdlib=True) config.target_run_simple_swift = make_simple_target_run() config.target_run_simple_swift_parameterized = make_simple_target_run(parameterized=True) + config.target_run_simple_leaks_swift_parameterized = make_simple_leaks_target_run(parameterized=True) config.target_run_stdlib_swift_parameterized = make_simple_target_run(stdlib=True, parameterized=True) config.target_run_simple_swiftgyb_parameterized = make_simple_target_run(gyb=True, parameterized=True) config.available_features.add('interpret') @@ -2273,6 +2274,11 @@ elif not kIsWindows: lit_config.note('Testing with the just-built libraries') lit_config.note('Library load path: {0}'.format(os.path.pathsep.join(target_stdlib_path))) + config.target_run_with_leaks = ( + "/usr/bin/env " + + construct_library_path_env(target_stdlib_path) + + " xcrun leaks -atExit -- " + + config.target_run) config.target_run = ( "/usr/bin/env " + construct_library_path_env(target_stdlib_path) + @@ -2308,6 +2314,16 @@ if not getattr(config, 'target_run_simple_swift', None): escape_for_substitute_captures(config.target_codesign), escape_for_substitute_captures(config.target_run)) ) + config.target_run_simple_leaks_swift_parameterized = SubstituteCaptures( + r"%%empty-directory(%%t) && " + r"%s %s %%s \1 -o %%t/a.out -module-name main && " + r"%s %%t/a.out && " + r"%s %%t/a.out" + % (escape_for_substitute_captures(config.target_build_swift), + escape_for_substitute_captures(mcp_opt), + escape_for_substitute_captures(config.target_codesign), + escape_for_substitute_captures(config.target_run_with_leaks)) + ) config.target_fail_simple_swift_parameterized = SubstituteCaptures( r"%%empty-directory(%%t) && " r"%s %s %%s \1 -o %%t/a.out -module-name main && " @@ -2483,6 +2499,8 @@ config.substitutions.append(('%target-run-simple-swiftgyb\(([^)]+)\)', config.substitutions.append(('%target-run-simple-swiftgyb', config.target_run_simple_swiftgyb)) config.substitutions.append(('%target-run-simple-swift\(([^)]+)\)', config.target_run_simple_swift_parameterized)) +config.substitutions.append(('%target-run-simple-leaks-swift\(([^)]+)\)', + config.target_run_simple_leaks_swift_parameterized)) config.substitutions.append(('%target-fail-simple-swift\(([^)]+)\)', config.target_fail_simple_swift_parameterized)) config.substitutions.append(('%target-run-stdlib-swift\(([^)]+)\)', From 750263f6f6aa3868343a60566a89d840f9f80bd7 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Mon, 8 May 2023 17:33:41 +0900 Subject: [PATCH 2/8] [Discarding] Don't leak retained "first error" task when retaining it --- include/swift/ABI/MetadataValues.h | 4 + .../Concurrency/DiscardingTaskGroup.swift | 40 +++--- stdlib/public/Concurrency/Task.cpp | 40 ++++-- stdlib/public/Concurrency/Task.swift | 32 +++++ stdlib/public/Concurrency/TaskGroup.cpp | 40 ++++-- stdlib/public/runtime/HeapObject.cpp | 4 + .../include/Runtime/Concurrency.h | 1 - .../Runtime/async_taskgroup_discarding.swift | 65 ++++++---- .../async_taskgroup_discarding_dontLeak.swift | 122 ++++++++++++++++++ ...roup_discarding_dontLeak_class_error.swift | 54 ++++++++ 10 files changed, 335 insertions(+), 67 deletions(-) create mode 100644 test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift create mode 100644 test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak_class_error.swift diff --git a/include/swift/ABI/MetadataValues.h b/include/swift/ABI/MetadataValues.h index ace93d8b4f9fe..f84b691ec272b 100644 --- a/include/swift/ABI/MetadataValues.h +++ b/include/swift/ABI/MetadataValues.h @@ -2334,6 +2334,7 @@ class TaskCreateFlags : public FlagSet { Task_InheritContext = 11, Task_EnqueueJob = 12, Task_AddPendingGroupTaskUnconditionally = 13, + Task_IsDiscardingTask = 14, }; explicit constexpr TaskCreateFlags(size_t bits) : FlagSet(bits) {} @@ -2360,6 +2361,9 @@ class TaskCreateFlags : public FlagSet { FLAGSET_DEFINE_FLAG_ACCESSORS(Task_AddPendingGroupTaskUnconditionally, addPendingGroupTaskUnconditionally, setAddPendingGroupTaskUnconditionally) + FLAGSET_DEFINE_FLAG_ACCESSORS(Task_IsDiscardingTask, + isDiscardingTask, + setIsDiscardingTask) }; /// Flags for schedulable jobs. diff --git a/stdlib/public/Concurrency/DiscardingTaskGroup.swift b/stdlib/public/Concurrency/DiscardingTaskGroup.swift index f2b531aafc35b..049a5692afa36 100644 --- a/stdlib/public/Concurrency/DiscardingTaskGroup.swift +++ b/stdlib/public/Concurrency/DiscardingTaskGroup.swift @@ -171,21 +171,21 @@ public struct DiscardingTaskGroup { #if SWIFT_STDLIB_TASK_TO_THREAD_MODEL_CONCURRENCY @available(*, unavailable, message: "Unavailable in task-to-thread concurrency model", renamed: "addTask(operation:)") #endif - public mutating func addTask( + public mutating func addTask( priority: TaskPriority? = nil, - operation: __owned @Sendable @escaping () async -> DiscardedResult + operation: __owned @Sendable @escaping () async -> Void ) { #if SWIFT_STDLIB_TASK_TO_THREAD_MODEL_CONCURRENCY let flags = taskCreateFlags( priority: priority, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: false, - addPendingGroupTaskUnconditionally: true + addPendingGroupTaskUnconditionally: true, isDiscardingTask: true ) #else let flags = taskCreateFlags( priority: priority, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: true + addPendingGroupTaskUnconditionally: true, isDiscardingTask: true ) #endif @@ -206,9 +206,9 @@ public struct DiscardingTaskGroup { #if SWIFT_STDLIB_TASK_TO_THREAD_MODEL_CONCURRENCY @available(*, unavailable, message: "Unavailable in task-to-thread concurrency model", renamed: "addTask(operation:)") #endif - public mutating func addTaskUnlessCancelled( + public mutating func addTaskUnlessCancelled( priority: TaskPriority? = nil, - operation: __owned @Sendable @escaping () async -> DiscardedResult + operation: __owned @Sendable @escaping () async -> Void ) -> Bool { let canAdd = _taskGroupAddPendingTask(group: _group, unconditionally: false) @@ -220,13 +220,13 @@ public struct DiscardingTaskGroup { let flags = taskCreateFlags( priority: priority, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: false, - addPendingGroupTaskUnconditionally: false + addPendingGroupTaskUnconditionally: false, isDiscardingTask: true ) #else let flags = taskCreateFlags( priority: priority, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: false + addPendingGroupTaskUnconditionally: false, isDiscardingTask: true ) #endif @@ -237,13 +237,13 @@ public struct DiscardingTaskGroup { } @_alwaysEmitIntoClient - public mutating func addTask( - operation: __owned @Sendable @escaping () async -> DiscardedResult + public mutating func addTask( + operation: __owned @Sendable @escaping () async -> Void ) { let flags = taskCreateFlags( priority: nil, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: true + addPendingGroupTaskUnconditionally: true, isDiscardingTask: true ) // Create the task in this group. @@ -260,8 +260,8 @@ public struct DiscardingTaskGroup { @available(*, unavailable, message: "Unavailable in task-to-thread concurrency model", renamed: "addTaskUnlessCancelled(operation:)") #endif @_alwaysEmitIntoClient - public mutating func addTaskUnlessCancelled( - operation: __owned @Sendable @escaping () async -> DiscardedResult + public mutating func addTaskUnlessCancelled( + operation: __owned @Sendable @escaping () async -> Void ) -> Bool { #if compiler(>=5.5) && $BuiltinCreateAsyncTaskInGroup let canAdd = _taskGroupAddPendingTask(group: _group, unconditionally: false) @@ -274,7 +274,7 @@ public struct DiscardingTaskGroup { let flags = taskCreateFlags( priority: nil, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: false + addPendingGroupTaskUnconditionally: false, isDiscardingTask: true ) // Create the task in this group. @@ -547,15 +547,15 @@ public struct ThrowingDiscardingTaskGroup { @available(*, unavailable, message: "Unavailable in task-to-thread concurrency model", renamed: "addTask(operation:)") #endif @_alwaysEmitIntoClient - public mutating func addTask( + public mutating func addTask( priority: TaskPriority? = nil, - operation: __owned @Sendable @escaping () async throws -> DiscardedResult + operation: __owned @Sendable @escaping () async throws -> Void ) { #if compiler(>=5.5) && $BuiltinCreateAsyncTaskInGroup let flags = taskCreateFlags( priority: priority, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: true + addPendingGroupTaskUnconditionally: true, isDiscardingTask: true ) // Create the task in this group. @@ -569,9 +569,9 @@ public struct ThrowingDiscardingTaskGroup { @available(*, unavailable, message: "Unavailable in task-to-thread concurrency model", renamed: "addTask(operation:)") #endif @_alwaysEmitIntoClient - public mutating func addTaskUnlessCancelled( + public mutating func addTaskUnlessCancelled( priority: TaskPriority? = nil, - operation: __owned @Sendable @escaping () async throws -> DiscardedResult + operation: __owned @Sendable @escaping () async throws -> Void ) -> Bool { #if compiler(>=5.5) && $BuiltinCreateAsyncTaskInGroup let canAdd = _taskGroupAddPendingTask(group: _group, unconditionally: false) @@ -584,7 +584,7 @@ public struct ThrowingDiscardingTaskGroup { let flags = taskCreateFlags( priority: priority, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: false + addPendingGroupTaskUnconditionally: false, isDiscardingTask: true ) // Create the task in this group. diff --git a/stdlib/public/Concurrency/Task.cpp b/stdlib/public/Concurrency/Task.cpp index 6e4e515fd61c4..1d4d57d40b022 100644 --- a/stdlib/public/Concurrency/Task.cpp +++ b/stdlib/public/Concurrency/Task.cpp @@ -940,6 +940,7 @@ static AsyncTaskAndContext swift_task_create_commonImpl( // be is the final hop. Store a signed null instead. initialContext->Parent = nullptr; + // FIXME: add discarding flag concurrency::trace::task_create( task, parent, group, asyncLet, static_cast(task->Flags.getPriority()), @@ -1060,21 +1061,40 @@ void swift::swift_task_run_inline(OpaqueValue *result, void *closureAFP, SWIFT_CC(swift) AsyncTaskAndContext swift::swift_task_create( - size_t taskCreateFlags, + size_t rawTaskCreateFlags, TaskOptionRecord *options, const Metadata *futureResultType, void *closureEntry, HeapObject *closureContext) { - FutureAsyncSignature::FunctionType *taskEntry; - size_t initialContextSize; - std::tie(taskEntry, initialContextSize) = + TaskCreateFlags taskCreateFlags(rawTaskCreateFlags); + + if (taskCreateFlags.isDiscardingTask()) { + ThinNullaryAsyncSignature::FunctionType *taskEntry; + size_t initialContextSize; + + std::tie(taskEntry, initialContextSize) = getAsyncClosureEntryPointAndContextSize< - FutureAsyncSignature, - SpecialPointerAuthDiscriminators::AsyncFutureFunction>(closureEntry); + ThinNullaryAsyncSignature, + SpecialPointerAuthDiscriminators::AsyncThinNullaryFunction>(closureEntry); + + return swift_task_create_common( + rawTaskCreateFlags, options, futureResultType, + reinterpret_cast(taskEntry), closureContext, + initialContextSize); - return swift_task_create_common( - taskCreateFlags, options, futureResultType, - reinterpret_cast(taskEntry), closureContext, - initialContextSize); + } else { + FutureAsyncSignature::FunctionType *taskEntry; + size_t initialContextSize; + + std::tie(taskEntry, initialContextSize) = + getAsyncClosureEntryPointAndContextSize< + FutureAsyncSignature, + SpecialPointerAuthDiscriminators::AsyncFutureFunction>(closureEntry); + + return swift_task_create_common( + rawTaskCreateFlags, options, futureResultType, + reinterpret_cast(taskEntry), closureContext, + initialContextSize); + } } #ifdef __ARM_ARCH_7K__ diff --git a/stdlib/public/Concurrency/Task.swift b/stdlib/public/Concurrency/Task.swift index ef852e7ca79bc..27de6afae4706 100644 --- a/stdlib/public/Concurrency/Task.swift +++ b/stdlib/public/Concurrency/Task.swift @@ -525,6 +525,38 @@ func taskCreateFlags( return bits } +/// Form task creation flags for use with the createAsyncTask builtins. +@available(SwiftStdlib 5.9, *) +@_alwaysEmitIntoClient +func taskCreateFlags( + priority: TaskPriority?, isChildTask: Bool, copyTaskLocals: Bool, + inheritContext: Bool, enqueueJob: Bool, + addPendingGroupTaskUnconditionally: Bool, + isDiscardingTask: Bool +) -> Int { + var bits = 0 + bits |= (bits & ~0xFF) | Int(priority?.rawValue ?? 0) + if isChildTask { + bits |= 1 << 8 + } + if copyTaskLocals { + bits |= 1 << 10 + } + if inheritContext { + bits |= 1 << 11 + } + if enqueueJob { + bits |= 1 << 12 + } + if addPendingGroupTaskUnconditionally { + bits |= 1 << 13 + } + if isDiscardingTask { + bits |= 1 << 14 + } + return bits +} + // ==== Task Creation ---------------------------------------------------------- @available(SwiftStdlib 5.1, *) extension Task where Failure == Never { diff --git a/stdlib/public/Concurrency/TaskGroup.cpp b/stdlib/public/Concurrency/TaskGroup.cpp index 67830e832e7b0..8790106b8253f 100644 --- a/stdlib/public/Concurrency/TaskGroup.cpp +++ b/stdlib/public/Concurrency/TaskGroup.cpp @@ -64,7 +64,7 @@ using namespace swift; -#if 0 +#if 1 #define SWIFT_TASK_GROUP_DEBUG_LOG(group, fmt, ...) \ fprintf(stderr, "[%#lx] [%s:%d][group(%p%s)] (%s) " fmt "\n", \ (unsigned long)Thread::current().platformThreadId(), \ @@ -228,7 +228,7 @@ class TaskGroupBase : public TaskGroupTaskStatusRecord { reinterpret_cast(fragment->getError()) : fragment->getStoragePtr(), /*successType=*/fragment->getResultType(), - /*task=*/asyncTask + /*retainedTask==*/asyncTask }; } @@ -1040,7 +1040,7 @@ static void fillGroupNextResult(TaskFutureWaitAsyncContext *context, case PollStatus::Error: { auto error = reinterpret_cast(result.storage); - fillGroupNextErrorResult(context, error); + fillGroupNextErrorResult(context, error); // FIXME: this specifically retains the error, but likely should not!??!!? return; } @@ -1146,7 +1146,10 @@ void AccumulatingTaskGroup::offer(AsyncTask *completedTask, AsyncContext *contex // will need to release in the other path. lock(); // TODO: remove fragment lock, and use status for synchronization - SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, completedTask:%p , status:%s", completedTask, statusString().c_str()); + SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, completedTask:%p (count:%d), status:%s", + completedTask, + swift_retainCount(completedTask), + statusString().c_str()); // Immediately increment ready count and acquire the status // @@ -1218,7 +1221,6 @@ void DiscardingTaskGroup::offer(AsyncTask *completedTask, AsyncContext *context) // We can do this, since in this mode there is no ready count to keep track of, // and we immediately discard the result. auto afterComplete = statusCompletePendingAssumeRelease(); - (void) afterComplete; const bool alreadyDecrementedStatus = true; SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, complete, status afterComplete:%s", afterComplete.to_string(this).c_str()); @@ -1291,6 +1293,14 @@ 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(completedTask->futureFragment()->getStoragePtr())); +// } } } else { // it wasn't the last pending task, and there is no-one to resume; @@ -1351,14 +1361,17 @@ void TaskGroupBase::resumeWaitingTask( // Run the task. auto result = PollResult::get(completedTask, hadErrorResult); SWIFT_TASK_GROUP_DEBUG_LOG(this, - "resume waiting DONE, task = %p, backup = %p, error:%d, complete with = %p, status = %s", - waitingTask, backup, hadErrorResult, completedTask, statusString().c_str()); + "resume waiting DONE, task = %p, error:%d, complete with = %p, status = %s", + waitingTask, hadErrorResult, completedTask, statusString().c_str()); auto waitingContext = static_cast( waitingTask->ResumeContext); + SWIFT_TASK_GROUP_DEBUG_LOG(this, "completed task %p", completedTask); + fillGroupNextResult(waitingContext, result); + SWIFT_TASK_GROUP_DEBUG_LOG(this, "completed task %p; AFTER FILL", completedTask); // Remove the child from the task group's running tasks list. // The parent task isn't currently running (we're about to wake @@ -1368,7 +1381,18 @@ void TaskGroupBase::resumeWaitingTask( // does a parent -> child traversal while recursively holding // locks) because we know that the child task is completed and // we can't be holding its locks ourselves. + 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) { + 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(result.storage); + swift_release(completedTask); // we need to do this if the error is a class + } _swift_tsan_acquire(static_cast(waitingTask)); // TODO: allow the caller to suggest an executor @@ -1377,7 +1401,7 @@ void TaskGroupBase::resumeWaitingTask( #endif /* SWIFT_CONCURRENCY_TASK_TO_THREAD_MODEL */ } else { SWIFT_TASK_GROUP_DEBUG_LOG(this, "CAS failed, task = %p, backup = %p, complete with = %p, status = %s", - waitingTask, backup, completedTask, statusString().c_str()); + waitingTask, completedTask, statusString().c_str()); } } } diff --git a/stdlib/public/runtime/HeapObject.cpp b/stdlib/public/runtime/HeapObject.cpp index 4065dddaca954..dd96ade47163d 100644 --- a/stdlib/public/runtime/HeapObject.cpp +++ b/stdlib/public/runtime/HeapObject.cpp @@ -526,6 +526,7 @@ 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); @@ -533,6 +534,7 @@ void swift::swift_nonatomic_release(HeapObject *object) { 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); @@ -796,6 +798,8 @@ 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); } diff --git a/stdlib/toolchain/Compatibility56/include/Runtime/Concurrency.h b/stdlib/toolchain/Compatibility56/include/Runtime/Concurrency.h index 5ba9fe3fd9669..eff5b075219f7 100644 --- a/stdlib/toolchain/Compatibility56/include/Runtime/Concurrency.h +++ b/stdlib/toolchain/Compatibility56/include/Runtime/Concurrency.h @@ -51,7 +51,6 @@ struct AsyncTaskAndContext { AsyncContext *InitialContext; }; - /// Caution: not all future-initializing functions actually throw, so /// this signature may be incorrect. using FutureAsyncSignature = diff --git a/test/Concurrency/Runtime/async_taskgroup_discarding.swift b/test/Concurrency/Runtime/async_taskgroup_discarding.swift index 27d74c7160c0c..2c0da0edc0c25 100644 --- a/test/Concurrency/Runtime/async_taskgroup_discarding.swift +++ b/test/Concurrency/Runtime/async_taskgroup_discarding.swift @@ -1,4 +1,4 @@ -// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) +// RUN: %target-run-simple-leaks-swift( -Xfrontend -disable-availability-checking -parse-as-library) // REQUIRES: concurrency // REQUIRES: executable_test @@ -10,29 +10,34 @@ // UNSUPPORTED: back_deployment_runtime import _Concurrency +import Darwin -struct Boom: Error { - let id: String +final class FIRST_PAYLOAD {} +final class SECOND_PAYLOAD {} + +final class FIRST_ERROR: Error { + let first: FIRST_PAYLOAD init(file: String = #fileID, line: UInt = #line) { - self.id = "\(file):\(line)" + first = .init() } - - init(id: String) { - self.id = id + deinit { + fputs("\(Self.self) deinit\n", stderr) } } -struct IgnoredBoom: Error {} +final class SECOND_ERROR: Error { + let second: SECOND_PAYLOAD -@discardableResult -func echo(_ i: Int) -> Int { i } -@discardableResult -func boom(file: String = #fileID, line: UInt = #line) throws -> Int { throw Boom(file: file, line: line) } + init(file: String = #fileID, line: UInt = #line) { + second = .init() + } -func shouldEqual(_ lhs: T, _ rhs: T) { - precondition(lhs == rhs, "'\(lhs)' did not equal '\(rhs)'") + deinit { + fputs("\(Self.self) deinit\n", stderr) + } } + func shouldStartWith(_ lhs: Any, _ rhs: Any) { let l = "\(lhs)" let r = "\(rhs)" @@ -43,22 +48,26 @@ func shouldStartWith(_ lhs: Any, _ rhs: Any) { @main struct Main { static func main() async { - for i in 0...1_000 { - do { - let got = try await withThrowingDiscardingTaskGroup() { group in - group.addTask { - echo(1) - } - group.addTask { - try boom() - } - - return 12 + do { + let got = try await withThrowingDiscardingTaskGroup() { group in + group.addTask { + 1 } - fatalError("(iteration:\(i)) expected error to be re-thrown, got: \(got)") - } catch { - shouldStartWith(error, "Boom(") + group.addTask { + throw FIRST_ERROR() + } + +// try? await Task.sleep(for: .seconds(1)) + + group.addTask { + throw SECOND_ERROR() + } + + return 12 } + fatalError("expected error to be re-thrown, got: \(got)") + } catch { + // shouldStartWith(error, "main.Boom") } } } diff --git a/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift b/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift new file mode 100644 index 0000000000000..28a7dbafaf345 --- /dev/null +++ b/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift @@ -0,0 +1,122 @@ +// 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 + +// REQUIRES: concurrency +// REQUIRES: executable_test +// REQUIRES: concurrency_runtime + +// UNSUPPORTED: back_deployment_runtime +// UNSUPPORTED: freestanding + +// FIXME: enable discarding taskgroup on windows; rdar://104762037 +// UNSUPPORTED: OS=windows-msvc + +import _Concurrency + +struct Boom: Error { + let id: String + + init(file: String = #fileID, line: UInt = #line) { + self.id = "\(file):\(line)" + } + + init(id: String) { + self.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 + } +} + +final class SomeClass: @unchecked Sendable { +//struct SomeClass: @unchecked Sendable { + let number: Int + init() { + self.number = 0 + } + init(number: Int) { + self.number = number + } +} + +// NOTE: Not as StdlibUnittest/TestSuite since these types of tests are unreasonably slow to load/debug. + +@main struct Main { + static func main() async { + _ = try? await withThrowingDiscardingTaskGroup() { group in + group.addTask { + throw Boom() + } + group.addTask { + SomeClass() // will be discarded + } + + return 12 + } + + // many ok + _ = try? await withThrowingDiscardingTaskGroup() { group in + for i in 0..<10 { + group.addTask { + SomeClass(number: i) // will be discarded + } + } + + return 12 + } + + // many throws + do { + let value = try await withThrowingDiscardingTaskGroup() { group in + for i in 0..<10 { + group.addTask { + throw BoomClass() // will be rethrown + } + } + + 12 // must be ignored + } + preconditionFailure("Should throw") + } catch { + precondition("\(error)" == "main.BoomClass", "error was: \(error)") + } + + // many errors, many values + _ = try? await withThrowingDiscardingTaskGroup() { group in + group.addTask { + SomeClass() // will be discarded + } + group.addTask { + SomeClass() // will be discarded + } + group.addTask { + SomeClass() // will be discarded + } + group.addTask { + throw Boom() + } + group.addTask { + throw Boom() + } + group.addTask { + throw Boom() + } + group.addTask { + throw Boom() + } + + return 12 + } + } +} diff --git a/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak_class_error.swift b/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak_class_error.swift new file mode 100644 index 0000000000000..b1b269a7ceb06 --- /dev/null +++ b/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak_class_error.swift @@ -0,0 +1,54 @@ +// 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 + +// REQUIRES: concurrency +// REQUIRES: executable_test +// REQUIRES: concurrency_runtime + +// UNSUPPORTED: back_deployment_runtime +// UNSUPPORTED: freestanding + +// FIXME: enable discarding taskgroup on windows; rdar://104762037 +// UNSUPPORTED: OS=windows-msvc + +import _Concurrency + +final class ClassBoom: Error { + let id: String + + init(file: String = #fileID, line: UInt = #line) { + self.id = "\(file):\(line)" + } + + init(id: String) { + self.id = id + } +} + +@main struct Main { + static func main() async { + + // many errors + _ = try? await withThrowingDiscardingTaskGroup() { group in + group.addTask { + throw ClassBoom() + } + group.addTask { + throw ClassBoom() + } + group.addTask { + throw ClassBoom() + } + group.addTask { + throw ClassBoom() + } + group.addTask { + 12 + } + + return 12 + } + } +} From f6f2e897b7e5bd73f441590fb4634900c05412d9 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 23 May 2023 13:57:15 +0200 Subject: [PATCH 3/8] make (normal) task group "dont leak" test use `leaks` and enable by default --- .../async_taskgroup_dontLeakTasks.swift | 47 +++++++------------ 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/test/Concurrency/Runtime/async_taskgroup_dontLeakTasks.swift b/test/Concurrency/Runtime/async_taskgroup_dontLeakTasks.swift index c31397e782ad9..9bbeef4e52368 100644 --- a/test/Concurrency/Runtime/async_taskgroup_dontLeakTasks.swift +++ b/test/Concurrency/Runtime/async_taskgroup_dontLeakTasks.swift @@ -1,52 +1,39 @@ -// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) 2>&1 | %FileCheck %s --dump-input=always +// 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 + // REQUIRES: executable_test // REQUIRES: concurrency -// REQUIRES: swift_task_debug_log // REQUIRES: concurrency_runtime // UNSUPPORTED: back_deployment_runtime -#if os(Linux) -import Glibc -#elseif os(Windows) -import MSVCRT -#else import Darwin -#endif + +final class Something { + let int: Int + init(int: Int) { + self.int = int + } +} func test_taskGroup_next() async { - // CHECK: creating task [[MAIN_TASK:0x.*]] with parent 0x0 - // CHECK: creating task [[GROUP_TASK_1:0x.*]] with parent [[MAIN_TASK]] - // CHECK: creating task [[GROUP_TASK_2:0x.*]] with parent [[MAIN_TASK]] - // CHECK: creating task [[GROUP_TASK_3:0x.*]] with parent [[MAIN_TASK]] - // CHECK: creating task [[GROUP_TASK_4:0x.*]] with parent [[MAIN_TASK]] - // CHECK: creating task [[GROUP_TASK_5:0x.*]] with parent [[MAIN_TASK]] - - _ = await withTaskGroup(of: Int.self, returning: Int.self) { group in - for n in 0..<5 { + let tasks = 5 + _ = await withTaskGroup(of: Something.self, returning: Int.self) { group in + for n in 0.. Date: Tue, 23 May 2023 15:17:06 +0200 Subject: [PATCH 4/8] back to println tests --- stdlib/public/Concurrency/TaskGroup.cpp | 45 +++++---- stdlib/public/runtime/HeapObject.cpp | 4 - .../Runtime/async_taskgroup_discarding.swift | 30 +++--- .../async_taskgroup_discarding_dontLeak.swift | 96 ++++++++++++------- ...roup_discarding_dontLeak_class_error.swift | 36 +++---- .../async_taskgroup_dontLeakTasks.swift | 14 ++- .../async_taskgroup_throw_recover.swift | 2 +- 7 files changed, 132 insertions(+), 95 deletions(-) diff --git a/stdlib/public/Concurrency/TaskGroup.cpp b/stdlib/public/Concurrency/TaskGroup.cpp index 8790106b8253f..614cd7d3abc42 100644 --- a/stdlib/public/Concurrency/TaskGroup.cpp +++ b/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(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(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(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(waitingTask)); diff --git a/stdlib/public/runtime/HeapObject.cpp b/stdlib/public/runtime/HeapObject.cpp index dd96ade47163d..4065dddaca954 100644 --- a/stdlib/public/runtime/HeapObject.cpp +++ b/stdlib/public/runtime/HeapObject.cpp @@ -526,7 +526,6 @@ 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); @@ -534,7 +533,6 @@ void swift::swift_nonatomic_release(HeapObject *object) { 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); @@ -798,8 +796,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); } diff --git a/test/Concurrency/Runtime/async_taskgroup_discarding.swift b/test/Concurrency/Runtime/async_taskgroup_discarding.swift index 2c0da0edc0c25..5dd297aac16d6 100644 --- a/test/Concurrency/Runtime/async_taskgroup_discarding.swift +++ b/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 @@ -10,31 +11,30 @@ // UNSUPPORTED: back_deployment_runtime 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)") } } @@ -54,20 +54,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 } } diff --git a/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift b/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift index 28a7dbafaf345..f41d29adf58b5 100644 --- a/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift +++ b/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 } } diff --git a/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak_class_error.swift b/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak_class_error.swift index b1b269a7ceb06..8227662a7b375 100644 --- a/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak_class_error.swift +++ b/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak_class_error.swift @@ -1,7 +1,4 @@ -// 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 // REQUIRES: concurrency // REQUIRES: executable_test @@ -20,10 +17,11 @@ final class ClassBoom: Error { init(file: String = #fileID, line: UInt = #line) { self.id = "\(file):\(line)" + print("INIT OF ClassBoom from \(id)") } - init(id: String) { - self.id = id + deinit { + print("DEINIT OF ClassBoom from \(id)") } } @@ -32,23 +30,17 @@ final class ClassBoom: Error { // many errors _ = try? await withThrowingDiscardingTaskGroup() { group in - group.addTask { - throw ClassBoom() - } - group.addTask { - throw ClassBoom() - } - group.addTask { - throw ClassBoom() - } - group.addTask { - throw ClassBoom() - } - group.addTask { - 12 - } - + group.addTask { throw ClassBoom() } + group.addTask { throw ClassBoom() } + group.addTask { throw ClassBoom() } + group.addTask { throw ClassBoom() } + group.addTask { 12 } return 12 + + // CHECK: DEINIT OF ClassBoom + // CHECK: DEINIT OF ClassBoom + // CHECK: DEINIT OF ClassBoom + // CHECK: DEINIT OF ClassBoom } } } diff --git a/test/Concurrency/Runtime/async_taskgroup_dontLeakTasks.swift b/test/Concurrency/Runtime/async_taskgroup_dontLeakTasks.swift index 9bbeef4e52368..1386a1e760242 100644 --- a/test/Concurrency/Runtime/async_taskgroup_dontLeakTasks.swift +++ b/test/Concurrency/Runtime/async_taskgroup_dontLeakTasks.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 // This test uses `leaks` which is only available on apple platforms; limit it to macOS: // REQUIRES: OS=macosx @@ -16,6 +17,10 @@ final class Something { init(int: Int) { self.int = int } + + deinit { + print("deinit, Something, int: \(int)") + } } func test_taskGroup_next() async { @@ -32,6 +37,13 @@ func test_taskGroup_next() async { sum += value.int } + + // CHECK-DAG: deinit, Something, int: 0 + // CHECK-DAG: deinit, Something, int: 1 + // CHECK-DAG: deinit, Something, int: 2 + // CHECK-DAG: deinit, Something, int: 3 + // CHECK-DAG: deinit, Something, int: 4 + return sum } } diff --git a/test/Concurrency/Runtime/async_taskgroup_throw_recover.swift b/test/Concurrency/Runtime/async_taskgroup_throw_recover.swift index f428b50d1ef47..28a741955582f 100644 --- a/test/Concurrency/Runtime/async_taskgroup_throw_recover.swift +++ b/test/Concurrency/Runtime/async_taskgroup_throw_recover.swift @@ -1,4 +1,4 @@ -// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s --dump-input=always +// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s // REQUIRES: executable_test // REQUIRES: concurrency From be18b86ce452119ea4b4752985566f66b4fd7df1 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Wed, 24 May 2023 18:26:46 +0200 Subject: [PATCH 5/8] less code duplication --- stdlib/public/Concurrency/Task.swift | 52 +++++-------------- stdlib/public/Concurrency/TaskGroup.swift | 38 +++++++------- stdlib/public/Concurrency/TaskSleep.swift | 3 +- .../Concurrency/TaskSleepDuration.swift | 3 +- 4 files changed, 38 insertions(+), 58 deletions(-) diff --git a/stdlib/public/Concurrency/Task.swift b/stdlib/public/Concurrency/Task.swift index 27de6afae4706..6988cbde7363a 100644 --- a/stdlib/public/Concurrency/Task.swift +++ b/stdlib/public/Concurrency/Task.swift @@ -200,7 +200,8 @@ extension Task where Failure == Error { let flags = taskCreateFlags(priority: priority, isChildTask: false, copyTaskLocals: true, inheritContext: true, enqueueJob: false, - addPendingGroupTaskUnconditionally: false) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) let (task, _) = Builtin.createAsyncTask(flags, work) _startTaskOnMainActor(task) return Task(task) @@ -222,7 +223,8 @@ extension Task where Failure == Never { let flags = taskCreateFlags(priority: priority, isChildTask: false, copyTaskLocals: true, inheritContext: true, enqueueJob: false, - addPendingGroupTaskUnconditionally: false) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) let (task, _) = Builtin.createAsyncTask(flags, work) _startTaskOnMainActor(task) return Task(task) @@ -503,36 +505,8 @@ struct JobFlags { func taskCreateFlags( priority: TaskPriority?, isChildTask: Bool, copyTaskLocals: Bool, inheritContext: Bool, enqueueJob: Bool, - addPendingGroupTaskUnconditionally: Bool -) -> Int { - var bits = 0 - bits |= (bits & ~0xFF) | Int(priority?.rawValue ?? 0) - if isChildTask { - bits |= 1 << 8 - } - if copyTaskLocals { - bits |= 1 << 10 - } - if inheritContext { - bits |= 1 << 11 - } - if enqueueJob { - bits |= 1 << 12 - } - if addPendingGroupTaskUnconditionally { - bits |= 1 << 13 - } - return bits -} - -/// Form task creation flags for use with the createAsyncTask builtins. -@available(SwiftStdlib 5.9, *) -@_alwaysEmitIntoClient -func taskCreateFlags( - priority: TaskPriority?, isChildTask: Bool, copyTaskLocals: Bool, - inheritContext: Bool, enqueueJob: Bool, - addPendingGroupTaskUnconditionally: Bool, - isDiscardingTask: Bool + addPendingGroupTaskUnconditionally: Bool, + isDiscardingTask: Bool ) -> Int { var bits = 0 bits |= (bits & ~0xFF) | Int(priority?.rawValue ?? 0) @@ -605,7 +579,8 @@ extension Task where Failure == Never { let flags = taskCreateFlags( priority: priority, isChildTask: false, copyTaskLocals: true, inheritContext: true, enqueueJob: true, - addPendingGroupTaskUnconditionally: false) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) // Create the asynchronous task. let (task, _) = Builtin.createAsyncTask(flags, operation) @@ -665,8 +640,8 @@ extension Task where Failure == Error { let flags = taskCreateFlags( priority: priority, isChildTask: false, copyTaskLocals: true, inheritContext: true, enqueueJob: true, - addPendingGroupTaskUnconditionally: false - ) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) // Create the asynchronous task future. let (task, _) = Builtin.createAsyncTask(flags, operation) @@ -724,7 +699,8 @@ extension Task where Failure == Never { let flags = taskCreateFlags( priority: priority, isChildTask: false, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: false) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) // Create the asynchronous task future. let (task, _) = Builtin.createAsyncTask(flags, operation) @@ -783,8 +759,8 @@ extension Task where Failure == Error { let flags = taskCreateFlags( priority: priority, isChildTask: false, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: false - ) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) // Create the asynchronous task future. let (task, _) = Builtin.createAsyncTask(flags, operation) diff --git a/stdlib/public/Concurrency/TaskGroup.swift b/stdlib/public/Concurrency/TaskGroup.swift index c8a7930d27f17..35414ec9d2072 100644 --- a/stdlib/public/Concurrency/TaskGroup.swift +++ b/stdlib/public/Concurrency/TaskGroup.swift @@ -272,14 +272,15 @@ public struct TaskGroup { let flags = taskCreateFlags( priority: priority, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: false, - addPendingGroupTaskUnconditionally: true + addPendingGroupTaskUnconditionally: true, + isDiscardingTask: false ) #else let flags = taskCreateFlags( priority: priority, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: true - ) + addPendingGroupTaskUnconditionally: true, + isDiscardingTask: false) #endif // Create the task in this group. @@ -314,14 +315,14 @@ public struct TaskGroup { let flags = taskCreateFlags( priority: priority, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: false, - addPendingGroupTaskUnconditionally: false - ) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) #else let flags = taskCreateFlags( priority: priority, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: false - ) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) #endif // Create the task in this group. @@ -354,8 +355,8 @@ public struct TaskGroup { let flags = taskCreateFlags( priority: nil, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: true - ) + addPendingGroupTaskUnconditionally: true, + isDiscardingTask: false) // Create the task in this group. _ = Builtin.createAsyncTaskInGroup(flags, _group, operation) @@ -394,8 +395,8 @@ public struct TaskGroup { let flags = taskCreateFlags( priority: nil, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: false - ) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) // Create the task in this group. _ = Builtin.createAsyncTaskInGroup(flags, _group, operation) @@ -682,7 +683,8 @@ public struct ThrowingTaskGroup { let flags = taskCreateFlags( priority: priority, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: true + addPendingGroupTaskUnconditionally: true, + isDiscardingTask: false ) // Create the task in this group. @@ -720,8 +722,8 @@ public struct ThrowingTaskGroup { let flags = taskCreateFlags( priority: priority, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: false - ) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) // Create the task in this group. _ = Builtin.createAsyncTaskInGroup(flags, _group, operation) @@ -756,8 +758,8 @@ public struct ThrowingTaskGroup { let flags = taskCreateFlags( priority: nil, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: true - ) + addPendingGroupTaskUnconditionally: true, + isDiscardingTask: false) // Create the task in this group. _ = Builtin.createAsyncTaskInGroup(flags, _group, operation) @@ -799,8 +801,8 @@ public struct ThrowingTaskGroup { let flags = taskCreateFlags( priority: nil, isChildTask: true, copyTaskLocals: false, inheritContext: false, enqueueJob: true, - addPendingGroupTaskUnconditionally: false - ) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) // Create the task in this group. _ = Builtin.createAsyncTaskInGroup(flags, _group, operation) diff --git a/stdlib/public/Concurrency/TaskSleep.swift b/stdlib/public/Concurrency/TaskSleep.swift index 043d4a3d31b9d..c84f5ba2f1b0d 100644 --- a/stdlib/public/Concurrency/TaskSleep.swift +++ b/stdlib/public/Concurrency/TaskSleep.swift @@ -240,7 +240,8 @@ extension Task where Success == Never, Failure == Never { let sleepTaskFlags = taskCreateFlags( priority: nil, isChildTask: false, copyTaskLocals: false, inheritContext: false, enqueueJob: false, - addPendingGroupTaskUnconditionally: false) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) let (sleepTask, _) = Builtin.createAsyncTask(sleepTaskFlags) { onSleepWake(wordPtr) } diff --git a/stdlib/public/Concurrency/TaskSleepDuration.swift b/stdlib/public/Concurrency/TaskSleepDuration.swift index 8981d91fa7819..5431926561937 100644 --- a/stdlib/public/Concurrency/TaskSleepDuration.swift +++ b/stdlib/public/Concurrency/TaskSleepDuration.swift @@ -58,7 +58,8 @@ extension Task where Success == Never, Failure == Never { let sleepTaskFlags = taskCreateFlags( priority: nil, isChildTask: false, copyTaskLocals: false, inheritContext: false, enqueueJob: false, - addPendingGroupTaskUnconditionally: false) + addPendingGroupTaskUnconditionally: false, + isDiscardingTask: false) let (sleepTask, _) = Builtin.createAsyncTask(sleepTaskFlags) { onSleepWake(wordPtr) } From 5ea671d6224d326fe01e0f39d816691df4c68a11 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Wed, 24 May 2023 18:55:34 +0200 Subject: [PATCH 6/8] remove extra debug statements --- stdlib/public/Concurrency/TaskGroup.cpp | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/stdlib/public/Concurrency/TaskGroup.cpp b/stdlib/public/Concurrency/TaskGroup.cpp index 614cd7d3abc42..1395dc10716ea 100644 --- a/stdlib/public/Concurrency/TaskGroup.cpp +++ b/stdlib/public/Concurrency/TaskGroup.cpp @@ -1152,9 +1152,8 @@ void AccumulatingTaskGroup::offer(AsyncTask *completedTask, AsyncContext *contex // will need to release in the other path. lock(); // TODO: remove fragment lock, and use status for synchronization - SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, completedTask:%p (count:%d), status:%s", + SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, completedTask:%p, status:%s", completedTask, - swift_retainCount(completedTask), statusString().c_str()); // Immediately increment ready count and acquire the status @@ -1251,7 +1250,6 @@ void DiscardingTaskGroup::offer(AsyncTask *completedTask, AsyncContext *context) break; case ReadyStatus::Error: SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, complete, resume with errorItem.task:%p", readyErrorItem.getTask()); - SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, complete, expect that it was extra retained %p", readyErrorItem.getTask()); resumeWaitingTask(readyErrorItem.getTask(), assumed, /*hadErrorResult=*/true, alreadyDecrementedStatus, @@ -1374,11 +1372,7 @@ void TaskGroupBase::resumeWaitingTask( auto waitingContext = static_cast( waitingTask->ResumeContext); - SWIFT_TASK_GROUP_DEBUG_LOG(this, "completed task %p", completedTask); - - fillGroupNextResult(waitingContext, result); - SWIFT_TASK_GROUP_DEBUG_LOG(this, "completed task %p; AFTER FILL", completedTask); // Remove the child from the task group's running tasks list. // The parent task isn't currently running (we're about to wake @@ -1388,16 +1382,10 @@ void TaskGroupBase::resumeWaitingTask( // does a parent -> child traversal while recursively holding // locks) because we know that the child task is completed and // we can't be holding its locks ourselves. - 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 (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 = completedTask->futureFragment()->getError(); swift_release(completedTask); } From cd2cf428d85e31cc8de33c6131e9cabcf4f4da96 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Thu, 25 May 2023 19:27:31 +0200 Subject: [PATCH 7/8] fix lit config on windows --- test/lit.cfg | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/test/lit.cfg b/test/lit.cfg index 571b49b391b01..1dd4897c6f52d 100644 --- a/test/lit.cfg +++ b/test/lit.cfg @@ -2314,16 +2314,17 @@ if not getattr(config, 'target_run_simple_swift', None): escape_for_substitute_captures(config.target_codesign), escape_for_substitute_captures(config.target_run)) ) - config.target_run_simple_leaks_swift_parameterized = SubstituteCaptures( - r"%%empty-directory(%%t) && " - r"%s %s %%s \1 -o %%t/a.out -module-name main && " - r"%s %%t/a.out && " - r"%s %%t/a.out" - % (escape_for_substitute_captures(config.target_build_swift), - escape_for_substitute_captures(mcp_opt), - escape_for_substitute_captures(config.target_codesign), - escape_for_substitute_captures(config.target_run_with_leaks)) - ) + if not kIsWindows: + config.target_run_simple_leaks_swift_parameterized = SubstituteCaptures( + r"%%empty-directory(%%t) && " + r"%s %s %%s \1 -o %%t/a.out -module-name main && " + r"%s %%t/a.out && " + r"%s %%t/a.out" + % (escape_for_substitute_captures(config.target_build_swift), + escape_for_substitute_captures(mcp_opt), + escape_for_substitute_captures(config.target_codesign), + escape_for_substitute_captures(config.target_run_with_leaks)) + ) config.target_fail_simple_swift_parameterized = SubstituteCaptures( r"%%empty-directory(%%t) && " r"%s %s %%s \1 -o %%t/a.out -module-name main && " @@ -2499,8 +2500,9 @@ config.substitutions.append(('%target-run-simple-swiftgyb\(([^)]+)\)', config.substitutions.append(('%target-run-simple-swiftgyb', config.target_run_simple_swiftgyb)) config.substitutions.append(('%target-run-simple-swift\(([^)]+)\)', config.target_run_simple_swift_parameterized)) -config.substitutions.append(('%target-run-simple-leaks-swift\(([^)]+)\)', - config.target_run_simple_leaks_swift_parameterized)) +if not kIsWindows: + config.substitutions.append(('%target-run-simple-leaks-swift\(([^)]+)\)', + config.target_run_simple_leaks_swift_parameterized)) config.substitutions.append(('%target-fail-simple-swift\(([^)]+)\)', config.target_fail_simple_swift_parameterized)) config.substitutions.append(('%target-run-stdlib-swift\(([^)]+)\)', From 1a4c4c94c47d19864c10595e29e65ab1caee1cfa Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Fri, 26 May 2023 09:13:44 +0200 Subject: [PATCH 8/8] relax ordering checks a bit, the deinits may not actually run in expected order; but as long as they all run we're happy --- .../async_taskgroup_discarding_dontLeak.swift | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift b/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift index f41d29adf58b5..a72a8376ba340 100644 --- a/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift +++ b/test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift @@ -67,8 +67,8 @@ final class SomeClass: @unchecked Sendable { 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 + // CHECK-DAG: deinit, id: race-boom-class + // CHECK-DAG: deinit, id: race-boom-class return 12 } @@ -80,12 +80,12 @@ final class SomeClass: @unchecked Sendable { 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 + // CHECK-DAG: deinit, id: many-ok + // CHECK-DAG: deinit, id: many-ok + // CHECK-DAG: deinit, id: many-ok + // CHECK-DAG: deinit, id: many-ok + // CHECK-DAG: deinit, id: many-ok + // CHECK-DAG: deinit, id: many-ok } return 12 @@ -101,12 +101,12 @@ final class SomeClass: @unchecked Sendable { } // 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 + // CHECK-DAG: deinit, id: many-error + // CHECK-DAG: deinit, id: many-error + // CHECK-DAG: deinit, id: many-error + // CHECK-DAG: deinit, id: many-error + // CHECK-DAG: deinit, id: many-error + // CHECK-DAG: deinit, id: many-error 12 // must be ignored } @@ -138,13 +138,13 @@ final class SomeClass: @unchecked Sendable { // 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 + // CHECK-DAG: deinit, id: mixed + // CHECK-DAG: deinit, id: mixed + // CHECK-DAG: deinit, id: mixed // three errors - // CHECK: deinit, id: mixed - // CHECK: deinit, id: mixed - // CHECK: deinit, id: mixed + // CHECK-DAG: deinit, id: mixed + // CHECK-DAG: deinit, id: mixed + // CHECK-DAG: deinit, id: mixed return 12 }