如何防止IDisposable在所有类中扩散?

141

从这些简单的类开始...

假设我有一组像这样的简单类:

class Bus
{
    Driver busDriver = new Driver();
}

class Driver
{
    Shoe[] shoes = { new Shoe(), new Shoe() };
}

class Shoe
{
    Shoelace lace = new Shoelace();
}

class Shoelace
{
    bool tied = false;
}

一辆 Bus 有一个 DriverDriver 有两只 Shoe,每个 Shoe 有一个 Shoelace。这都很傻。

向 Shoelace 添加 IDisposable 对象

后来我决定对 Shoelace 进行某些操作时可以使用多线程,因此我添加了一个 EventWaitHandle 用于线程间通信。现在 Shoelace 如下:

class Shoelace
{
    private AutoResetEvent waitHandle = new AutoResetEvent(false);
    bool tied = false;
    // ... other stuff ..
}

在Shoelace中实现IDisposable

但是现在Microsoft的FxCop会抱怨:"在'Shoelace'上实现IDisposable,因为它创建了以下IDisposable类型的成员:'EventWaitHandle'"。

好吧,我在Shoelace上实现了IDisposable,我的整洁小类变成了这个可怕的混乱:

class Shoelace : IDisposable
{
    private AutoResetEvent waitHandle = new AutoResetEvent(false);
    bool tied = false;
    private bool disposed = false;

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    ~Shoelace()
    {
        Dispose(false);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (!this.disposed)
        {
            if (disposing)
            {
                if (waitHandle != null)
                {
                    waitHandle.Close();
                    waitHandle = null;
                }
            }
            // No unmanaged resources to release otherwise they'd go here.
        }
        disposed = true;
    }
}

正如评论者所指出的,由于Shoelace本身没有非托管资源,因此我可以使用更简单的dispose实现而无需使用Dispose(bool)和析构函数:

class Shoelace : IDisposable
{
    private AutoResetEvent waitHandle = new AutoResetEvent(false);
    bool tied = false;

    public void Dispose()
    {
        if (waitHandle != null)
        {
            waitHandle.Close();
            waitHandle = null;
        }
        GC.SuppressFinalize(this);
    }
}

观看IDisposable的可怕传播

好了,这个问题得到解决。但是现在FxCop会抱怨Shoe创建Shoelace,所以Shoe也必须成为IDisposable

而且Driver创建Shoe,所以Driver也必须成为IDisposableBus创建Driver,所以Bus也必须成为IDisposable,以此类推。

突然间,我对Shoelace做出的小改变就让我需要做很多工作,我的老板也在想为什么我需要检查Bus来修改Shoelace

问题

如何防止IDisposable的传播,但仍确保您的非托管对象得到正确释放?


11
一个非常好的问题,我认为答案是尽量减少它们的使用,并尝试让高级 IDisposables 的生命周期短暂,但这并不总是可能的(特别是当这些 IDisposables 是由于与 C++ DLL 或类似内容的交互而必需时)。期待答案。 - annakata
好的丹,我已经更新了问题,展示了在Shoelace上实现IDisposable的两种方法。 - GrahamS
我通常不太相信依赖其他类的实现细节来保护自己。如果我可以轻松地防止这种情况,那么没有冒险的必要。也许我过于谨慎,或者可能只是因为我作为一名C程序员工作了太长时间,但我宁愿采取爱尔兰式的方法:“确保没问题”。 :) - GrahamS
@Dan:仍然需要进行空值检查,以确保对象本身不为 null,否则对 waitHandle.Dispose() 的调用将引发 NullReferenceException 异常。 - Scott Dorman
1
无论如何,您实际上仍应使用“在Shoelace上实现IDisposable”部分中显示的Dispose(bool)方法,因为那是完整的模式(减去终结器)。仅因为一个类实现了IDisposable并不意味着它需要一个终结器。 - Scott Dorman
@Scott Dorman:我强烈不同意。 "dispose-pattern" 是微软曾经想出的最糟糕的东西之一。拥有其他可处理类的类通常不应直接拥有非托管资源并且应该是密封的。=>只需实现Dispose()即可。即使它们被设计为基类,也没有理由实现处理模式-在这种情况下,只需实现Dispose虚拟即可。因为在我看来,没有借口(也没有必要)使用直接拥有非托管资源的内容来扩展类。 - Paul Groke
7个回答

36

您无法真正“防止”IDisposable的传播。有些类需要被处理,比如AutoResetEvent,而最有效的方式是在Dispose()方法中处理它们,以避免终结器的开销。但是这个方法必须要被调用,所以和你的示例一样,封装或包含IDisposable的类必须处理这些内容,因此它们也必须是可处置的,等等。唯一的避免方法是:

  • 在可能的情况下避免使用IDisposable类,在单个位置锁定或等待事件,在单个位置保留昂贵的资源等
  • 仅在需要时创建它们,并在之后立即处理它们(使用using模式)

在某些情况下,可以忽略IDisposable,因为它支持可选情况。例如,WaitHandle实现IDisposable以支持命名互斥量。如果没有使用名称,则Dispose方法不执行任何操作。MemoryStream是另一个例子,它不使用系统资源,其Dispose实现也不执行任何操作。仔细思考是否正在使用非托管资源可能会很有启发性。检查.net库的可用源代码或使用反编译器也可以。


7
“你可以忽略它,因为当前的实现没有做任何事情”的建议是不好的,因为未来版本可能会在Dispose中执行一些重要操作,那时你就会遇到难以跟踪的泄漏问题。” - Andy
1
保留昂贵的资源,并不意味着IDisposable一定是昂贵的。事实上,这个示例使用了一个轻量级的等待句柄,可以说是非常"轻量级"了。 - markmnl

20

在正确性方面,如果父对象创建并拥有一个必须被处理的子对象,那么你无法通过对象关系来防止IDisposable的传播。在这种情况下,FxCop是正确的,父对象必须是IDisposable。

你可以做的是避免将IDisposable添加到对象层次结构中的叶子类中。虽然这不总是一项容易的任务,但这是一项有趣的练习。从逻辑上讲,没有理由让一个鞋带成为可处理对象。因此,在这里添加一个WaitHandle的方法不如在使用它的时候建立ShoeLace和WaitHandle之间的关联。最简单的方法是通过一个Dictionary实例实现。

如果能够通过地图将WaitHandle移入松散关联,则可以在WaitHandle实际使用的点断开这个链条。


2
在那里建立一个关联感觉非常奇怪。这个AutoResetEvent是Shoelace实现中的私有对象,因此我认为公开它是不正确的。 - GrahamS
@GrahamS,我不是说要公开暴露它。我是说能否将其移动到系鞋带的位置。也许如果系鞋带是如此复杂的任务,应该有一个系鞋带类。 - JaredPar
1
JaredPar,你能否用一些代码片段来扩展你的回答?我可能举的例子有点牵强,但我想象Shoelace会创建并启动Tyer线程,该线程在waitHandle.WaitOne()处耐心等待。当Shoelace希望线程开始绑扎时,它会调用waitHandle.Set()。 - GrahamS
1
对此点赞。我认为,仅仅Shoelace被并发调用而其他的没有被调用可能会很奇怪。想一想应该将哪些内容合并成一个根聚合。在这种情况下,我的看法是它应该是Bus,尽管我不熟悉这个领域。因此,在这种情况下,Bus应该包含waithandle和所有针对Bus及其所有子对象的操作将被同步。 - Johannes Gustafsson

16
为了防止 IDisposable 的扩散,你应该尝试将一次性对象的使用封装在一个方法内部。 尝试以不同的方式设计Shoelace
class Shoelace { 
  bool tied = false; 

  public void Tie() {

    using (var waitHandle = new AutoResetEvent(false)) {

      // you can even pass the disposable to other methods
      OtherMethod(waitHandle);

      // or hold it in a field (but FxCop will complain that your class is not disposable),
      // as long as you take control of its lifecycle
      _waitHandle = waitHandle;
      OtherMethodThatUsesTheWaitHandleFromTheField();

    } 

  }
} 

等待句柄的作用域仅限于Tie方法,该类不需要具有可处理字段,因此本身不需要被处理。

由于等待句柄是Shoelace内部的实现细节,因此它不应以任何方式更改其公共接口,例如在声明中添加新接口。当您不再需要可处置字段时会发生什么情况?您会删除IDisposable声明吗? 如果您考虑Shoelace抽象,您很快就会意识到它不应被基础设施依赖项(如IDisposable)所污染。 IDisposable 应该保留给其抽象封装调用确定性清理的资源的类; 即对于需要处置的类而言,这是其抽象的一部分。


1
我完全同意Shoelace的实现细节不应该污染公共接口,但我的观点是很难避免。你在这里提出的建议并不总是可行的:AutoResetEvent()的目的是在线程之间通信,因此它的范围通常会超出单个方法。 - GrahamS
@GrahamS:这就是为什么我建议尝试以这种方式设计它。您还可以将可处理对象传递给其他方法。只有当类的外部调用控制可处理对象的生命周期时,才会出现问题。我会相应地更新答案。 - Jordão
2
抱歉,我知道你可以传递一次性使用的对象,但我仍然不认为这种方法可行。在我的例子中,AutoResetEvent 用于在同一个类中的不同线程之间进行通信,因此它必须是成员变量。你不能将其作用域限制在一个方法中。(例如,想象一个线程只是通过在 waitHandle.WaitOne() 上阻塞等待一些工作。然后主线程调用 shoelace.Tie() 方法,该方法只是执行 waitHandle.Set() 然后立即返回)。 - GrahamS

3

我认为如果你的设计过于紧密耦合,就没有技术方法可以防止IDisposable的传播。那么我们应该怀疑这种设计是否正确。

在你的例子中,我认为让鞋拥有鞋带是有道理的,也许司机应该拥有他/她的鞋子。但是,公交车不应该拥有司机。通常情况下,公交车司机不会跟随公交车一起报废 :) 对于司机和鞋子的情况,司机很少自己制作鞋子,这意味着他们并不真正“拥有”它们。

另一种替代设计可能是:

class Bus
{
   IDriver busDriver = null;
   public void SetDriver(IDriver d) { busDriver = d; }
}

class Driver : IDriver
{
   IShoePair shoes = null;
   public void PutShoesOn(IShoePair p) { shoes = p; }
}

class ShoePairWithDisposableLaces : IShoePair, IDisposable
{
   Shoelace lace = new Shoelace();
}

class Shoelace : IDisposable
{
   ...
}

新设计比较复杂,需要额外的类来实例化和处理鞋子和司机的具体实例,但这种复杂性是解决问题所固有的。好消息是,为了处理鞋带而将公交车报废的做法不再必要。

2
这并没有真正解决原始问题。它可能会让 FxCop 保持安静,但仍然应该处理 Shoelace 的释放。 - AndrewS
我认为你所做的一切都是把问题转移到了别处。 ShoePairWithDisposableLaces现在拥有Shoelace(),因此它也必须被制作为IDisposable——那么谁来处理这双鞋的处理?你打算让某个IoC容器来处理吗? - GrahamS
@AndrewS: 的确,仍然需要处理司机、鞋子和鞋带等物品的丢弃问题,但这不应该由公交车来发起。@GrahamS: 你说得对,我把问题移到了别处,因为我认为它属于别处。通常情况下,会有一个类实例化鞋子,并负责处理鞋子的丢弃。我已经编辑了代码,使ShoePairWithDisposableLaces也可以被处理掉。 - Joh
@Joh:谢谢。正如你所说,将其移动到其他地方的问题在于会创建更多的复杂性。如果ShoePairWithDisposableLaces是由其他类创建的,那么当司机使用完鞋子后,不是必须通知该类以便它可以正确地Dispose()它们吗? - GrahamS
1
@Graham:是的,需要某种通知机制。例如,可以使用基于引用计数的处理策略。这会增加一些复杂性,但正如你可能知道的那样,“没有免费的午餐” :) - Joh

3
有趣的是,如果将 Driver 定义如上所示:
class Driver
{
    Shoe[] shoes = { new Shoe(), new Shoe() };
}

Shoe被定义为IDisposable时,FxCop(v1.36)不会提示Driver也应该是IDisposable

但是,如果它被定义为如下所示:

class Driver
{
    Shoe leftShoe = new Shoe();
    Shoe rightShoe = new Shoe();
}

然后它会抱怨。

我怀疑这只是FxCop的限制,而不是解决方案,因为在第一个版本中,Shoe实例仍然由Driver创建,并且仍然需要以某种方式进行处理。


2
如果Show实现了IDisposable接口,在字段初始化器中创建它是危险的。有趣的是,FXCop不允许这样的初始化,因为在C#中,唯一安全的方法是使用ThreadStatic变量(非常丑陋)。在vb.net中,可以安全地初始化IDisposable对象而不使用ThreadStatic变量。代码仍然很丑陋,但没有那么可怕。我希望微软能提供一种安全使用字段初始化器的好方法。 - supercat
1
@supercat:谢谢。你可以解释一下为什么它是危险的吗?我猜,如果在一个Shoe构造函数中抛出异常,那么shoes可能会被留在一个棘手的状态? - GrahamS
3
除非知道某种类型的IDisposable可以安全地被放弃,否则应确保每个创建的对象都被Dispose。如果在对象的字段初始化器中创建了一个IDisposable,但在对象完全构造之前抛出了异常,则该对象及其所有字段将被放弃。即使对象的构建被包装在使用“try”块检测构建失败的工厂方法中,工厂也很难获取需要处理的对象的引用。 - supercat
3
在C#中,我知道处理这个问题的唯一方法是使用ThreadStatic变量来保留一个对象列表,如果当前构造函数抛出异常,则需要处理这些对象。然后每个字段初始化程序在创建每个对象时都将其注册,工厂在成功完成时清除该列表,并在未被清除时使用“finally”子句从列表中Dispose所有项。 这将是可行的方法,但ThreadStatic变量很难看。在vb.net中,可以使用类似的技术而不需要任何ThreadStatic变量。 - supercat

3

当您将组合或聚合与可处置类混合使用时,基本上会发生这种情况。如前所述,第一种解决方法是将waitHandle从shoelace中重构出来。

话虽如此,当您没有非托管资源时,可以大大简化可处置模式。(我仍在寻找官方参考文献。)

但是,您可以省略析构函数和GC.SuppressFinalize(this);,并且可能需要清理virtual void Dispose(bool disposing)。


谢谢Henk。如果我将waitHandle的创建从Shoelace中移除,那么仍然有某人需要在某个地方Dispose它,那么这个某人如何知道Shoelace已经完成了对它的使用(并且Shoelace没有将其传递给其他类)? - GrahamS
你需要移动的不仅是waithandle的创建,还有它的“责任”。jaredPar的答案提供了一个有趣的思路开端。 - H H
本博客介绍了如何实现IDisposable接口,并特别讨论了如何通过避免在单个类中混合托管和非托管资源来简化任务。http://blog.stephencleary.com/2009/08/how-to-implement-idisposable-and.html - PaulS

1
使用控制反转怎么样?
class Bus
{
    private Driver busDriver;

    public Bus(Driver busDriver)
    {
        this.busDriver = busDriver;
    }
}

class Driver
{
    private Shoe[] shoes;

    public Driver(Shoe[] shoes)
    {
        this.shoes = shoes;
    }
}

class Shoe
{
    private Shoelace lace;

    public Shoe(Shoelace lace)
    {
        this.lace = lace;
    }
}

class Shoelace
{
    bool tied;
    private AutoResetEvent waitHandle;

    public Shoelace(bool tied, AutoResetEvent waitHandle)
    {
        this.tied = tied;
        this.waitHandle = waitHandle;
    }
}

class Program
{
    static void Main(string[] args)
    {
        using (var leftShoeWaitHandle = new AutoResetEvent(false))
        using (var rightShoeWaitHandle = new AutoResetEvent(false))
        {
            var bus = new Bus(new Driver(new[] {new Shoe(new Shoelace(false, leftShoeWaitHandle)),new Shoe(new Shoelace(false, rightShoeWaitHandle))}));
        }
    }
}

现在你已经把问题留给了调用者,Shoelace不能依赖于等待句柄在需要时可用。 - Andy
@andy,“Shoelace can't depend on the wait handle being available when it needs it”是什么意思?它在构造函数中传递。 - Darragh
在将自动重置事件交给Shoelace之前,可能会有其他东西干扰它的状态,并且它可能以错误的状态启动;在Shoelace执行其任务时,可能会有其他东西干扰ARE的状态,导致Shoelace做错事情。这就是你只锁定私有成员的原因。 "一般来说,..避免锁定超出您代码控制范围的实例" https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/lock-statement#remarks - Andy
我同意只锁定私有成员。但是似乎等待句柄不同。实际上,将它们传递给对象似乎更有用,因为需要使用相同的实例来跨线程通信。尽管如此,我认为IoC为OP的问题提供了一个有用的解决方案。 - Darragh
考虑到 OP 的示例将 waithandle 作为私有成员,因此可以合理地假设该对象期望对其进行独占访问。将句柄在实例外部可见会违反这一点。通常 IoC 可以用于此类事情,但在涉及线程时不行。 - Andy
1
此外,Shoelace 可以逃离 using 作用域,而资源将在对象使用它时被 Dispose,仍然可以引用该对象。 - OwnageIsMagic

网页内容由stack overflow 提供, 点击上面的
可以查看英文原文,
原文链接