solve leak, must destroy before dealloc

Author: ktoso

lib/IRGen/GenDistributed.cpp

@@ -495,12 +495,8 @@ void DistributedAccessor::decodeArgument(unsigned argumentIdx,
   case ParameterConvention::Pack_Inout:
     llvm_unreachable("pack parameters are not supported");
 
-  case ParameterConvention::Direct_Guaranteed: {
-    fprintf(stderr, "[%s:%d](%s) Direct_Guaranteed\n", __FILE_NAME__, __LINE__, __FUNCTION__);
-    LLVM_FALLTHROUGH;
-  }
+  case ParameterConvention::Direct_Guaranteed:
   case ParameterConvention::Direct_Unowned: {
-    fprintf(stderr, "[%s:%d](%s) UNOWNED\n", __FILE_NAME__, __LINE__, __FUNCTION__);
     auto paramTy = param.getSILStorageInterfaceType();
     Address eltPtr = IGF.Builder.CreateElementBitCast(
         resultValue.getAddress(), IGM.getStorageType(paramTy));
@@ -587,6 +583,13 @@ void DistributedAccessor::emitLoadOfWitnessTables(llvm::Value *witnessTables,
 }
 
 void DistributedAccessor::emitReturn(llvm::Value *errorValue) {
+  // Destroy loaded arguments.
+  // This MUST be done before deallocating, as otherwise we'd try to swift_release freed memory,
+  // which will be a no-op, however that also would mean we never drop retain counts to 0 and miss to run deinitializers of classes!
+  llvm::for_each(LoadedArguments, [&](const auto &argInfo) {
+    emitDestroyCall(IGF, argInfo.second, argInfo.first);
+  });
+
   // Deallocate all of the copied arguments. Since allocations happened
   // on stack they have to be deallocated in reverse order.
   {
@@ -596,13 +599,6 @@ void DistributedAccessor::emitReturn(llvm::Value *errorValue) {
     }
   }
 
-  // Destroy loaded arguments.
-  for (auto argInfo = LoadedArguments.rbegin();
-       argInfo != LoadedArguments.rend(); ++argInfo) {
-
-    emitDestroyCall(IGF, argInfo->second, argInfo->first);
-  }
-
   Explosion voidResult;
 
   Explosion error;

test/Distributed/Runtime/distributed_actor_func_calls_decoded_args_deinit.swift

@@ -39,7 +39,6 @@ struct S<T> : Codable {
 
 distributed actor Greeter {
   distributed func test1<T>(_ value: SomeClass<T>) {
-    print("RETAIN COUNT IN [TEST1]: \(_swift_retainCount(value))")
   }
   distributed func test2<T>(_ value: S<T>) {}
 }
@@ -51,11 +50,11 @@ func test() async throws {
   let local = Greeter(actorSystem: system)
   let ref = try Greeter.resolve(id: local.id, using: system)
 
-  var value = SomeClass<Int>()
-  try await ref.test1(value)
-  print("RETAIN COUNT: \(_swift_retainCount(value))")
-//  _swift_release(value) // FIXME: we're missing a release!
-  print("RETAIN COUNT: \(_swift_retainCount(value))")
+  try await ref.test1(SomeClass<Int>())
+  // CHECK: SomeClass: init
+  // CHECK: SomeClass: deinit
+
+  try await ref.test2(S(data: SomeClass<Int>()))
   // CHECK: SomeClass: init
   // CHECK: SomeClass: deinit
 }
@@ -65,8 +64,3 @@ func test() async throws {
     try! await test()
   }
 }
-
-@_silgen_name("swift_release")
-func _swift_release(_: AnyObject)
-@_silgen_name("swift_retainCount")
-func _swift_retainCount(_: AnyObject) -> Int

test/Distributed/distributed_actor_thunk_doesnt_leak_class_arguments.swift

@@ -1,5 +1,5 @@
 // RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/Inputs/FakeDistributedActorSystems.swift
-// RUN: %target-swift-frontend -module-name no_to_arg_leaks -emit-irgen -disable-availability-checking -I %t 2>&1 %s | %IRGenFileCheck %s -check-prefix CHECK-%target-import-type --dump-input=always
+// RUN: %target-swift-frontend -module-name no_to_arg_leaks -emit-irgen -disable-availability-checking -I %t 2>&1 %s | %IRGenFileCheck %s -check-prefix CHECK-%target-import-type
 
 // UNSUPPORTED: back_deploy_concurrency
 // REQUIRES: concurrency
@@ -25,18 +25,20 @@ struct S<T> : Codable {
 
 distributed actor Greeter {
   // CHECK-LABEL: define linkonce_odr hidden swifttailcc void @"$s15no_to_arg_leaks7GreeterC5test1yyAA9SomeClassCyxGYaKlFTETF"
-  // CHECK: [[ARG_ADDR:%.*]] = bitcast i8* {{.*}} to %T15no_to_arg_leaks9SomeClassC**
+  // CHECK: [[ARG_ADDR:%.*]] = bitcast i8* [[PARAM:%.*]] to %T15no_to_arg_leaks9SomeClassC**
   // CHECK: %destroy = bitcast i8* {{.*}} to void (%swift.opaque*, %swift.type*)*
   // CHECK-NEXT: [[OPAQUE_ARG_ADDR:%.*]] = bitcast %T15no_to_arg_leaks9SomeClassC** [[ARG_ADDR]] to %swift.opaque*
   // CHECK-NEXT: call void %destroy(%swift.opaque* noalias [[OPAQUE_ARG_ADDR]], %swift.type* %arg_type)
+  // CHECK-NEXT: call swiftcc void @swift_task_dealloc(i8* [[PARAM]])
   distributed func test1<T>(_: SomeClass<T>) {
   }
 
   // CHECK-LABEL: define linkonce_odr hidden swifttailcc void @"$s15no_to_arg_leaks7GreeterC5test2yyAA1SVyxGYaKlFTETF"
-  // CHECK: [[ARG_ADDR:%.*]] = bitcast i8* {{.*}} to %T15no_to_arg_leaks1SV*
+  // CHECK: [[ARG_ADDR:%.*]] = bitcast i8* [[PARAM:%.*]] to %T15no_to_arg_leaks1SV*
   // CHECK: %destroy = bitcast i8* {{.*}} to void (%swift.opaque*, %swift.type*)*
   // CHECK-NEXT: [[OPAQUE_ARG_ADDR:%.*]] = bitcast %T15no_to_arg_leaks1SV* [[ARG_ADDR]] to %swift.opaque*
   // CHECK-NEXT: call void %destroy(%swift.opaque* noalias [[OPAQUE_ARG_ADDR]], %swift.type* %arg_type)
+  // CHECK-NEXT: call swiftcc void @swift_task_dealloc(i8* [[PARAM]])
   distributed func test2<T>(_: S<T>) {}
 }