WIP: Inject Owned

[jnm2]

@YairHalberstadt I get this email from my fork repo on every push. Any idea how to disable TOC generator in my fork?

[YairHalberstadt]

@jnm2 pull the latest master - should hopefully fix the issue.

[jnm2]

@YairHalberstadt I think I'm back to implementing the actual change when I get a chance to come back to this. Should I start a PR for some/all of the commits before the last one to keep it separate?

[YairHalberstadt]

Whatever's easiest for you - I'm ok reviewing them altogether or separately

[jnm2]

So this is what I'm going to try to do next.

-disposeActions_func_0_1 = new global::System.Collections.Concurrent.ConcurrentBag<global::System.Action>();
 func_0_1 = () =>
 {
     global::C c_1_2;
     global::B b_1_1;
     global::StrongInject.Owned<global::B> owned_1_0;
     c_1_2 = new global::C();
     try
     {
         b_1_1 = new global::B(c: c_1_2);
         try
         {
-            owned_1_0 = new global::StrongInject.Owned<global::B>(b_1_1, );
+            owned_1_0 = new global::StrongInject.Owned<global::B>(b_1_1, () =>
+            {
+                ((global::System.IDisposable)b_1_1).Dispose();
+                ((global::System.IDisposable)c_1_2).Dispose();
+            });
         }
         catch
         {
             ((global::System.IDisposable)b_1_1).Dispose();
             throw;
         }
     }
     catch
     {
         ((global::System.IDisposable)c_1_2).Dispose();
         throw;
     }

-    disposeActions_func_0_1.Add(() =>
-    {
-        ((global::System.IDisposable)b_1_1).Dispose();
-        ((global::System.IDisposable)c_1_2).Dispose();
-    });
     return owned_1_0;
 };

This is for:

using System;
using StrongInject;

[Register(typeof(A))]
[Register(typeof(B))]
[Register(typeof(C))]
public partial class Container : IContainer<A>
{
}

public record A(Func<Owned<B>> b);
public record B(C c) : IDisposable { public void Dispose() { } }
public record C : IDisposable { public void Dispose() { } };

[jnm2]

@YairHalberstadt Deciding not to dispose Owned<T> instances after the requesting component's Dispose/Async method completes means that we won't have a way to clean up Owned<T> instances if the requesting component throws in the constructor after invoking the func:

try
{
    a_0_0 = new global::A(b: func_0_1);
}
catch
{
    // This can't free any Owned<T> instances that `A..ctor` caused to exist before it threw an exception,
    // because we don't dispose Owned<T> in disposeActions_func_0_1.
    foreach (var disposeAction in disposeActions_func_0_1)
        disposeAction();
    throw;
}

[jnm2]

Think I'm possibly done, just need to write a ton of tests and maybe find interactions that don't work.

global::System.Func<global::StrongInject.Owned<global::B>> func_0_1;
global::A a_0_0;
func_0_1 = () =>
{
    global::C c_1_1;
    global::B b_1_0;
    global::StrongInject.Owned<global::B> owned_1_0;
    c_1_1 = new global::C();
    try
    {
        b_1_0 = new global::B(c: c_1_1);
    }
    catch
    {
        ((global::System.IDisposable)c_1_1).Dispose();
        throw;
    }

    owned_1_0 = new global::StrongInject.Owned<global::B>(b_1_0, () =>
    {
        ((global::System.IDisposable)b_1_0).Dispose();
        ((global::System.IDisposable)c_1_1).Dispose();
    });
    return owned_1_0;
};
a_0_0 = new global::A(b: func_0_1);

[YairHalberstadt]

Deciding not to dispose Owned instances after the requesting component's Dispose/Async method completes means that we won't have a way to clean up Owned instances if the requesting component throws in the constructor after invoking the func

I don't think that's a problem. The moment I pass you an Owned, you're responsible for disposing it. If I pass you a Func<Owned> then if the Func throws, I will clear up, but if it returns, you are responsible for disposing the Owned even if you throw in the same method, and even if that method is called as part of dependency resolution.

[jnm2]

I think I'm finished the implementation except for investigating deduplicating of local functions when Owned of a given type is used more than once, including everything we've discussed so far. I'll also need to see if there are any more corner cases worth testing, including ones that should possibly get diagnostics, and also write tests of the behavior of the generated code.

Here's the code to compare to the local functions sample you created (https://github.com/YairHalberstadt/stronginject/pull/143#discussion_r687037505):

Click to expand

```cs if (Disposed) throw new global::System.ObjectDisposedException(nameof(Container)); global::StrongInject.Owned owned_0_1; global::C c_0_3; global::A a_0_0; global::StrongInject.Owned CreateOwnedB_2() { global::C c_0_1; global::B b_0_0; c_0_1 = new global::C(); try { b_0_0 = new global::B(c: c_0_1); } catch { ((global::System.IDisposable)c_0_1).Dispose(); throw; } return new global::StrongInject.Owned(b_0_0, () => { ((global::System.IDisposable)b_0_0).Dispose(); ((global::System.IDisposable)c_0_1).Dispose(); }); } owned_0_1 = CreateOwnedB_2(); c_0_3 = new global::C(); try { a_0_0 = new global::A(b: owned_0_1, c: c_0_3); } catch { ((global::System.IDisposable)c_0_3).Dispose(); throw; } ```

[YairHalberstadt]

can you pull from main please to fix the tests? Thanks

[jnm2]

Yep, the diff should look a lot cleaner now! Hopefully I'll get to the remaining comments soon.

[jnm2]

Silly thing, can I start a new PR as the author so I can find it more easily in the future? I was very confused here for three minutes.

[YairHalberstadt]

Of course, go for it

[jnm2]

Looks like I can't create a PR as long as this one is open without using a new branch name. Oh well.

[jnm2]

@YairHalberstadt Do you want to either link this to https://github.com/YairHalberstadt/stronginject/issues/140 or close the PR and let me set one up and write the PR title and link it?

[YairHalberstadt]

Closing so you can open a new PR

[jnm2]

@YairHalberstadt Cool, https://github.com/YairHalberstadt/stronginject/pull/149 is set up and ready to continue.