From f223ba02951c7437649c7eb08ab684d9915a62c0 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Wed, 24 Nov 2021 16:28:42 -0500 Subject: [PATCH] WIP: initialize earlier --- .../Deployment_ReadOrRegisterResource.cs | 10 +--------- sdk/dotnet/Pulumi/Resources/Resource.cs | 20 ++++++++++++++----- .../Serialization/OutputCompletionSource.cs | 8 +++++--- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/sdk/dotnet/Pulumi/Deployment/Deployment_ReadOrRegisterResource.cs b/sdk/dotnet/Pulumi/Deployment/Deployment_ReadOrRegisterResource.cs index 6c516ec2b..297646668 100644 --- a/sdk/dotnet/Pulumi/Deployment/Deployment_ReadOrRegisterResource.cs +++ b/sdk/dotnet/Pulumi/Deployment/Deployment_ReadOrRegisterResource.cs @@ -22,17 +22,9 @@ namespace Pulumi // finished. Otherwise, we might actually read and get the result back *prior* to the // object finishing initializing. Note: this is not a speculative concern. This is // something that does happen and has to be accounted for. - // - // IMPORTANT! We have to make sure we run 'OutputCompletionSource.InitializeOutputs' - // synchronously directly when `resource`'s constructor runs since this will set all of - // the `[Output(...)] Output` properties. We need those properties assigned by the - // time the base 'Resource' constructor finishes so that both derived classes and - // external consumers can use the Output properties of `resource`. - var completionSources = OutputCompletionSource.InitializeOutputs(resource); - _runner.RegisterTask( $"{nameof(IDeploymentInternal.ReadOrRegisterResource)}: {resource.GetResourceType()}-{resource.GetResourceName()}", - CompleteResourceAsync(resource, remote, newDependency, args, options, completionSources)); + CompleteResourceAsync(resource, remote, newDependency, args, options, resource.CompletionSources)); } private async Task<(string urn, string id, Struct data, ImmutableDictionary> dependencies)> ReadOrRegisterResourceAsync( diff --git a/sdk/dotnet/Pulumi/Resources/Resource.cs b/sdk/dotnet/Pulumi/Resources/Resource.cs index cc06cadad..0d291a589 100644 --- a/sdk/dotnet/Pulumi/Resources/Resource.cs +++ b/sdk/dotnet/Pulumi/Resources/Resource.cs @@ -20,33 +20,33 @@ namespace Pulumi /// The child resources of this resource. We use these (only from a ComponentResource) to /// allow code to dependOn a ComponentResource and have that effectively mean that it is /// depending on all the CustomResource children of that component. - /// + /// /// Important! We only walk through ComponentResources.They're the only resources that /// serve as an aggregation of other primitive(i.e.custom) resources.While a custom resource /// can be a parent of other resources, we don't want to ever depend on those child /// resource. If we do, it's simple to end up in a situation where we end up depending on a /// child resource that has a data cycle dependency due to the data passed into it. An /// example of how this would be bad is: - /// + /// /// /// var c1 = new CustomResource("c1"); /// var c2 = new CustomResource("c2", { parentId = c1.id }, { parent = c1 }); /// var c3 = new CustomResource("c3", { parentId = c1.id }, { parent = c1 }); /// - /// + /// /// The problem here is that 'c2' has a data dependency on 'c1'. If it tries to wait on /// 'c1' it will walk to the children and wait on them.This will mean it will wait on 'c3'. /// But 'c3' will be waiting in the same manner on 'c2', and a cycle forms. This normally /// does not happen with ComponentResources as they do not have any data flowing into /// them.The only way you would be able to have a problem is if you had this sort of coding /// pattern: - /// + /// /// /// var c1 = new ComponentResource("c1"); /// var c2 = new CustomResource("c2", { parentId = c1.urn }, { parent: c1 }); /// var c3 = new CustomResource("c3", { parentId = c1.urn }, { parent: c1 }); /// - /// + /// /// However, this would be pretty nonsensical as there is zero need for a custom resource to /// ever need to reference the urn of a component resource. So it's acceptable if that sort /// of pattern failed in practice. @@ -103,6 +103,8 @@ namespace Pulumi /// internal readonly string? _version; + internal readonly ImmutableDictionary CompletionSources; + /// /// Creates and registers a new resource object. is the fully /// qualified type token and is the "name" part to use in creating a @@ -128,6 +130,7 @@ namespace Pulumi _name = ""; _protect = false; _providers = ImmutableDictionary.Empty; + CompletionSources = ImmutableDictionary.Empty; return; } @@ -137,6 +140,13 @@ namespace Pulumi if (string.IsNullOrEmpty(name)) throw new ArgumentException("'name' cannot be null or empty.", nameof(name)); + // Initialize all Output properties crucially including + // `Urn` to non-nil Output values, record + // `CompletionSources`. This needs to happen before + // partially initialized `this` is exposed to other + // threads via `parentResource.ChildResources`. + CompletionSources = OutputCompletionSource.InitializeOutputs(this); + // Before anything else - if there are transformations registered, invoke them in order // to transform the properties and options assigned to this resource. var parent = type == Stack._rootPulumiStackTypeName diff --git a/sdk/dotnet/Pulumi/Serialization/OutputCompletionSource.cs b/sdk/dotnet/Pulumi/Serialization/OutputCompletionSource.cs index 2d091ec76..6e5220b8d 100644 --- a/sdk/dotnet/Pulumi/Serialization/OutputCompletionSource.cs +++ b/sdk/dotnet/Pulumi/Serialization/OutputCompletionSource.cs @@ -17,7 +17,7 @@ namespace Pulumi.Serialization void TrySetException(Exception exception); void TrySetDefaultResult(bool isKnown); - + void SetStringValue(string value, bool isKnown); void SetValue(OutputData data); } @@ -72,13 +72,15 @@ namespace Pulumi.Serialization if (!propType.IsConstructedGenericType || propType.GetGenericTypeDefinition() != typeof(Output<>)) { - throw new InvalidOperationException($"{propFullName} was not an Output"); + continue; + // throw new InvalidOperationException($"{propFullName} was not an Output"); } var setMethod = prop.DeclaringType!.GetMethod("set_" + prop.Name, BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance); if (setMethod == null) { - throw new InvalidOperationException($"{propFullName} did not have a 'set' method"); + continue; + //throw new InvalidOperationException($"{propFullName} did not have a 'set' method"); } var outputTypeArg = propType.GenericTypeArguments.Single();