[TaskLocal] Crash on inapropriate use within task group

Author: ktoso

include/swift/ABI/MetadataKind.def

@@ -93,10 +93,6 @@ METADATAKIND(Task,
 METADATAKIND(Job,
              3 | MetadataKindIsNonType | MetadataKindIsRuntimePrivate)
 
-/// A heap-allocated task-local value `TaskLocal::Item`.
-METADATAKIND(TaskLocalHeapItem,
-             4 | MetadataKindIsNonType | MetadataKindIsRuntimePrivate)
-
 // getEnumeratedMetadataKind assumes that all the enumerated values here
 // will be <= LastEnumeratedMetadataKind.
 

include/swift/ABI/TaskLocal.h

@@ -28,8 +28,6 @@ struct SwiftError;
 class TaskStatusRecord;
 class TaskGroup;
 
-extern FullMetadata<HeapMetadata> taskLocalHeapItemHeapMetadata;
-
 // ==== Task Locals Values ---------------------------------------------------
 
 class TaskLocal {
@@ -46,9 +44,6 @@ class TaskLocal {
     /// lookups by skipping empty parent tasks during get(), and explained
     /// in depth in `createParentLink`.
     IsParent = 0b01,
-
-    /// The next item is "unstructured" and must be ref-counted.
-    ThisItemIsHeapItem = 0b10
   };
 
   /// Values must match `TaskLocalInheritance` declared in `TaskLocal.swift`.
@@ -133,10 +128,6 @@ class TaskLocal {
       return static_cast<NextLinkType>(next & statusMask);
     }
 
-    bool isHeapItem() const {
-      return getNextLinkType() == NextLinkType::ThisItemIsHeapItem;
-    }
-
     /// Item does not contain any actual value, and is only used to point at
     /// a specific parent item.
     bool isEmpty() const {
@@ -147,14 +138,13 @@ class TaskLocal {
     OpaqueValue *getStoragePtr() {
       fprintf(stderr, "[%s:%d] (%s) GET STORAGE\n", __FILE__, __LINE__, __FUNCTION__);
       return reinterpret_cast<OpaqueValue *>(
-        reinterpret_cast<char *>(this) + storageOffset(isHeapItem(), valueType));
+        reinterpret_cast<char *>(this) + storageOffset(valueType));
     }
 
     /// Compute the offset of the storage from the base of the item.
-    static size_t storageOffset(bool isHeapItem, const Metadata *valueType) {
+    static size_t storageOffset(const Metadata *valueType) {
       size_t offset = sizeof(Item);
 
-
       if (valueType) {
         size_t alignment = valueType->vw_alignment();
         return (offset + alignment - 1) & ~(alignment - 1);
@@ -164,54 +154,15 @@ class TaskLocal {
     }
 
     /// Determine the size of the item given a particular value type.
-    static size_t itemSize(bool isHeapItem, const Metadata *valueType) {
-      size_t offset = storageOffset(isHeapItem, valueType);
-      size_t headerSize = isHeapItem ? sizeof(HeapObject) : 0;
-      offset += headerSize;
+    static size_t itemSize(const Metadata *valueType) {
+      size_t offset = storageOffset(valueType);
       if (valueType) {
         offset += valueType->vw_size();
       }
       return offset;
     }
   };
 
-  /// Heap allocated version of TaskLocal::Item.
-  /// It is used in limited un-structured concurrency cases, where the task
-  /// local must be stored on the heap for the time being.
-  ///
-  /// MUST HAVE THE SAME LAYOUT AS TaskLocal::Item.
-  /// FIXME: how to actually make this work...
-  ///        get the address of "after heap object" and cast that to Item?
-  class HeapItem : public HeapObject, public Item {
-  public:
-
-    explicit HeapItem(const HeapMetadata *metadata = &taskLocalHeapItemHeapMetadata)
-        : HeapObject(metadata),
-          Item() {}
-
-    explicit HeapItem(const HeapObject *key, const Metadata *valueType)
-        : HeapItem(&taskLocalHeapItemHeapMetadata, key, valueType) {}
-
-    explicit HeapItem(const HeapMetadata *metadata,
-                      const HeapObject *key, const Metadata *valueType)
-        : HeapObject(metadata),
-          Item(key, valueType) {}
-
-
-    /// Retrieve a pointer to the `Item` that is part of this object.
-    TaskLocal::Item *getItem() {
-      fprintf(stderr, "[%s:%d] (%s) GET ITEM PTR, heapItem:%p\n", __FILE__, __LINE__, __FUNCTION__, this);
-      return reinterpret_cast<TaskLocal::Item *>(
-          reinterpret_cast<char *>(this) + itemOffset());
-    }
-
-    /// Compute the offset of `Item` part of this object.
-    static size_t itemOffset() {
-      size_t offset = sizeof(HeapObject);
-      return offset;
-    }
-  };
-
   class Storage {
     friend class TaskLocal::Item;
   private:

include/swift/Runtime/Concurrency.h

@@ -333,6 +333,19 @@ void swift_asyncLet_wait_throwing(OpaqueValue *,
 SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
 void swift_asyncLet_end(AsyncLet *alet);
 
+/// Returns true if the currently executing AsyncTask has a
+/// 'TaskGroupTaskStatusRecord' present.
+///
+/// This can be called from any thread.
+///
+/// Its Swift signature is
+///
+/// \code
+/// func swift_taskGroup_hasTaskGroupRecord()
+/// \endcode
+SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
+bool swift_taskGroup_hasTaskGroupRecord();
+
 /// Add a status record to a task.  The record should not be
 /// modified while it is registered with a task.
 ///
@@ -406,6 +419,12 @@ SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
 void swift_task_removeCancellationHandler(
     CancellationNotificationStatusRecord *record);
 
+/// Report error about attempting to bind a task-local value from an illegal context.
+SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
+void swift_task_reportIllegalTaskLocalBindingWithinWithTaskGroup(
+    const unsigned char *file, uintptr_t fileLength,
+    bool fileIsASCII, uintptr_t line);
+
 /// Get a task local value from the passed in task. Its Swift signature is
 ///
 /// \code

stdlib/public/CompatibilityOverride/CompatibilityOverrideConcurrency.def

@@ -218,6 +218,13 @@ OVERRIDE_TASK_GROUP(taskGroup_addPending, bool,
                     swift::, (TaskGroup *group, bool unconditionally),
                     (group, unconditionally))
 
+
+OVERRIDE_TASK_LOCAL(task_reportIllegalTaskLocalBindingWithinWithTaskGroup, void,
+                    SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift), swift::,
+                    (const unsigned char *file, uintptr_t fileLength,
+                        bool fileIsASCII, uintptr_t line),
+                    (file, fileLength, fileIsASCII, line))
+
 OVERRIDE_TASK_LOCAL(task_localValuePush, void,
                     SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift),
                     swift::,

stdlib/public/Concurrency/TaskGroup.swift

@@ -817,6 +817,10 @@ func _taskGroupIsCancelled(group: Builtin.RawPointer) -> Bool
 @_silgen_name("swift_taskGroup_wait_next_throwing")
 func _taskGroupWaitNext<T>(group: Builtin.RawPointer) async throws -> T?
 
+@available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *)
+@_silgen_name("swift_task_hasTaskGroupStatusRecord")
+func _taskHasTaskGroupStatusRecord() -> Bool
+
 @available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *)
 enum PollStatus: Int {
   case empty   = 0

stdlib/public/Concurrency/TaskGroupPrivate.h

@@ -22,13 +22,6 @@
 
 namespace swift {
 
-//void _swift_taskgroup_offer(AsyncTask *completed,
-//                            AsyncContext *context,
-//                            ExecutorRef executor);
-//
-//void _swift_taskgroup_cancelAll(AsyncTask *completed,
-//                                TaskGroup *group);
-
 } // end namespace swift
 
 #endif