Skip to content

[DiscardingTG] Undo pointer auth workaround; fix memory leaks #65613

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 26, 2023
Merged

[DiscardingTG] Undo pointer auth workaround; fix memory leaks #65613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
44aff27
[lit] introduce target-run-simple-leaks-swift to run with `leaks`
ktoso May 8, 2023
cad608e
[Discarding] Don't leak retained "first error" task when retaining it
ktoso May 8, 2023
b039691
make (normal) task group "dont leak" test use `leaks` and enable by d…
ktoso May 23, 2023
9f5a707
back to println tests
ktoso May 23, 2023
d82de55
less code duplication
ktoso May 24, 2023
760cb44
remove extra debug statements
ktoso May 24, 2023
e3d4b55
fix lit config on windows
ktoso May 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/swift/ABI/MetadataValues.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2334,6 +2334,7 @@ class TaskCreateFlags : public FlagSet<size_t> {
Task_InheritContext = 11,
Task_EnqueueJob = 12,
Task_AddPendingGroupTaskUnconditionally = 13,
Task_IsDiscardingTask = 14,
};

explicit constexpr TaskCreateFlags(size_t bits) : FlagSet(bits) {}
Expand All @@ -2360,6 +2361,9 @@ class TaskCreateFlags : public FlagSet<size_t> {
FLAGSET_DEFINE_FLAG_ACCESSORS(Task_AddPendingGroupTaskUnconditionally,
addPendingGroupTaskUnconditionally,
setAddPendingGroupTaskUnconditionally)
FLAGSET_DEFINE_FLAG_ACCESSORS(Task_IsDiscardingTask,
isDiscardingTask,
setIsDiscardingTask)
};

/// Flags for schedulable jobs.
Expand Down
40 changes: 20 additions & 20 deletions stdlib/public/Concurrency/DiscardingTaskGroup.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<DiscardedResult>(
public mutating func addTask(
priority: TaskPriority? = nil,
operation: __owned @Sendable @escaping () async -> DiscardedResult
Copy link
Contributor Author

@ktoso ktoso May 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire change to () -> T was a hotfix to temporarily unbreak but it is very wrong, we must get back to -> Void

✅ This fixes the (b) leaks the by undoing what introduced them in the first place.

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

Expand All @@ -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<DiscardedResult>(
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)

Expand All @@ -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

Expand All @@ -237,13 +237,13 @@ public struct DiscardingTaskGroup {
}

@_alwaysEmitIntoClient
public mutating func addTask<DiscardedResult>(
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.
Expand All @@ -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<DiscardedResult>(
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)
Expand All @@ -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.
Expand Down Expand Up @@ -547,15 +547,15 @@ public struct ThrowingDiscardingTaskGroup<Failure: Error> {
@available(*, unavailable, message: "Unavailable in task-to-thread concurrency model", renamed: "addTask(operation:)")
#endif
@_alwaysEmitIntoClient
public mutating func addTask<DiscardedResult>(
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.
Expand All @@ -569,9 +569,9 @@ public struct ThrowingDiscardingTaskGroup<Failure: Error> {
@available(*, unavailable, message: "Unavailable in task-to-thread concurrency model", renamed: "addTask(operation:)")
#endif
@_alwaysEmitIntoClient
public mutating func addTaskUnlessCancelled<DiscardedResult>(
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)
Expand All @@ -584,7 +584,7 @@ public struct ThrowingDiscardingTaskGroup<Failure: Error> {
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.
Expand Down
40 changes: 30 additions & 10 deletions stdlib/public/Concurrency/Task.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t>(task->Flags.getPriority()),
Expand Down Expand Up @@ -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()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the pointer auth dance for the nullary function type:

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<TaskContinuationFunction *>(taskEntry), closureContext,
initialContextSize);

return swift_task_create_common(
taskCreateFlags, options, futureResultType,
reinterpret_cast<TaskContinuationFunction *>(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<TaskContinuationFunction *>(taskEntry), closureContext,
initialContextSize);
}
}

#ifdef __ARM_ARCH_7K__
Expand Down
26 changes: 17 additions & 9 deletions stdlib/public/Concurrency/Task.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Success, Error>(task)
Expand All @@ -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)
Expand Down Expand Up @@ -503,7 +505,8 @@ struct JobFlags {
func taskCreateFlags(
priority: TaskPriority?, isChildTask: Bool, copyTaskLocals: Bool,
inheritContext: Bool, enqueueJob: Bool,
addPendingGroupTaskUnconditionally: Bool
addPendingGroupTaskUnconditionally: Bool,
isDiscardingTask: Bool
) -> Int {
var bits = 0
bits |= (bits & ~0xFF) | Int(priority?.rawValue ?? 0)
Expand All @@ -522,6 +525,9 @@ func taskCreateFlags(
if addPendingGroupTaskUnconditionally {
bits |= 1 << 13
}
if isDiscardingTask {
bits |= 1 << 14
}
return bits
}

Expand Down Expand Up @@ -573,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)
Expand Down Expand Up @@ -633,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)
Expand Down Expand Up @@ -692,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)
Expand Down Expand Up @@ -751,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)
Expand Down
Loading