-1

I am trying to use a reference that is defined outside of a delegate I have defined like so (simplified version of what I am dealing with):

private void DoStuff(int objectKey)
{
    MyObject myObject;

    if (this.concurrentDictionary.TryRemove(objectKey, out myObject))
    {
        Action<IList<IEvent>> eventsCompletedDelegate (eventList) =>
        {
            // Do work ...

            myObject.DoSomething();
        };

        ExecuteStuffAsync(eventsCompletedDelegate);
    }
}

The problem is that eventsCompletedDelegate is executed asynchronously (some time after ExecuteStuffAsync is called). I want to be able to access myObject from within the closure but by the time the delegate is invoked the local myObject reference will be disposed. Is there a way to pass in myObject into the delegate so that it will still be available by the time the delegate is invoked?

Andrew
  • 1,581
  • 3
  • 18
  • 31
  • For us to answer this question you'd have to show us the code that manages the lifetime of `myObject`. – Enigmativity Mar 18 '16 at 03:53
  • @Enigmativity I modified the code above. – Andrew Mar 18 '16 at 03:58
  • @Andrew the code above still doesn't show any disposal of your object, nor does it show any asynchronous code execution within ExecuteStuffAsync. I can't reproduce your disposal. – David L Mar 18 '16 at 04:02
  • I don't call Dispose() explicitly at any point before the delegate is invoked. So does that I mean I am safe? Why does Resharper think I am accessing a disposed object? – Andrew Mar 18 '16 at 04:05
  • It's probably warning you that you MAY be accessing a disposed object, which is not the same thing. – David L Mar 18 '16 at 04:05
  • I am worried about whether it's possible for it to be garbage collected before the delegate is invoked? I am passing myObject into other objects which are then dealt with in the ExecuteStuffAsync above (I left that code out for simplicity). So then I can always assume myObject would not be disposed then? – Andrew Mar 18 '16 at 04:08
  • No, you can't safely assume that. There could be a case where the object would be disposed of (in the case of a database connection). Your best bet is still to pass in a local variable copy of the reference to the delegate. Just keep in mind that you should also release that reference to avoid a memory leak. – David L Mar 18 '16 at 04:11
  • @Andrew - Your code still doesn't show the lifetime of the `myObject` instances. The lifetime is from when you call `new MyObject` until (in this case) you call `myObject.Dispose()` (or it might be in a `using` statement which implicitly calls `.Dispose()`). – Enigmativity Mar 18 '16 at 04:47
  • @Andrew - Just a clarification - when you say "by the time the delegate is invoked the local myObject reference will be disposed" do you mean your reference will be "disposed" or "garbage collected"? – Enigmativity Mar 18 '16 at 04:49
  • @Enigmativity Disposed, not garbage collected. – Andrew Mar 18 '16 at 04:58
  • @Andrew - Then you need to show us the code that I asked for - from construction to disposal. – Enigmativity Mar 18 '16 at 05:52

2 Answers2

0

No, myObject won't be disposed since there is a reference to it from eventsCompletedDelegate (variable capture). This is a common cause for memory leaking.

Community
  • 1
  • 1
George Chen
  • 6,592
  • 5
  • 21
  • 23
  • It might easily be disposed. What you're talking about is being garbage collected. Two different things. – Enigmativity Mar 18 '16 at 03:52
  • what do you mean disposed here? first, in this case, it won't be garbage collected. second: if it is an Idisposable and have Close() called, then "passing this myObject into the delegate" doesn't help anything. – George Chen Mar 18 '16 at 03:59
  • If the OP's code is calling `.Dispose()` then holding a reference to it in `eventCompletedDelegate` will not prevent `.Dispose()` being called, but it will prevent the object from being garbage collected. – Enigmativity Mar 18 '16 at 04:45
-1

If you create an object reference x of myObject and pass it instead to your action, your "copied" reference x will still be available in the callback and will not have been garbage collected.

This would output "reached DoSomething 1".

However, if the reference is disposed, as noted by Enigmativity, this will modify the copied reference as well, calling disposed. this would instead output "reached DoSomething 0". There isn't a way to prevent this other than ensuring that myObject is not disposed until the callback is invoked.

void Main()
{
    concurrentDictionary[1] = new MyObject(1);
    DoStuff(1);
}

private ConcurrentDictionary<int, MyObject> concurrentDictionary = 
    new ConcurrentDictionary<int, UserQuery.MyObject>();

private async void DoStuff(int objectKey)
{
    MyObject myObject;

    if (this.concurrentDictionary.TryRemove(objectKey, out myObject))
    {
        Action<MyObject> eventsCompletedDelegate = (objectRef) =>
        {
            objectRef.DoSomething();
        };
        var x = myObject;
        // myObject.Dispose(); // will set _id to 0 if called
        myObject = null;

        await ExecuteStuffAsync(() => eventsCompletedDelegate(x));
    }
}

public async Task ExecuteStuffAsync(Action callback)
{
    await Task.Delay(1000);
    callback();
}


public class MyObject : IDisposable
{
    private int _id;    
    public MyObject(int id)
    {
        _id = id;   
    }

    public void Dispose() { _id = 0; }

    public void DoSomething()
    {
        Console.WriteLine("reached DoSomething " + _id);
    }
}
David L
  • 32,885
  • 8
  • 62
  • 93
  • No, this is just wrong. `MyObject` is a `class`. So taking a copy of its reference means that if you dispose `myObject` you're also disposing `x`. Also calling `.Dispose()` has absolutely nothing to do with garbage collection. They are two entirely different things. – Enigmativity Mar 18 '16 at 04:51