在使用块内部进行返回是否可行？

Question

我正在进行代码审查，并发现了很多以下格式的代码：

public MyResponse MyMethod(string arg)
{
    using (Tracer myTracer = new Tracer(Constants.TraceLog))
    {
        MyResponse abc = new MyResponse();

        // Some code

        return abc;
    }
}

运行代码分析时，我收到了一个CA2000警告Microsoft.Reliability

应该将代码改成：

public MyResponse MyMethod(string arg)
{
   MyResponse abc = new MyResponse();

   using (Tracer myTracer = new Tracer(Constants.TraceLog))
   {
       // Some code
   }
   return abc;
}

或者说这并不重要吗？

编辑

报告警告的行是：

MyResponse abc = new MyResponse();

MyResponse是一个标准数据集。

完整的错误信息如下：

警告 150 CA2000：Microsoft.Reliability：在方法 'xxxxx（Guid，Guid）'中，对象'MyResponse'没有在所有异常路径上被处理。 在所有引用超出范围之前调用System.IDisposable.Dispose对对象'MyResponse'进行处理。

- Shiraz Bhaiji

---

MyResponse 实现了 IDisposable 接口吗？ - Jon

在我看来，这个警告并没有什么意义...我看不出第二个版本为什么会更好。 - Thomas Levesque

@Anthony：不要那么肯定。请看我的回答以获取更多细节。 - Daniel Hilgarth

你为什么要向我们隐瞒警告的文本内容？我们并非心灵读者；如果你想要获得错误信息的诊断，请公开发布信息。 - Eric Lippert

@Eric：通常情况下，我会同意你的看法，但是CA2000和他的例子非常清晰，你不觉得吗？ - Daniel Hilgarth

Answer 1

不，这没关系。

using语句隐式生成的finally块用于处理回收将在您放置return的任何位置都执行。

您确定CA2000与myTracer有关而不是abc吗？我猜测警告出现是因为MyResponse实现了IDisposable接口，而您没有在返回之前处理好abc的回收。(无论如何，您建议的重写对警告应该没有影响。)

- LukeH

---

@smartcaveman - 这个问题以“或者这不重要吗？”结尾，因此这个答案开头的陈述是完全正确的。 - Richard Szalay

@Richard，我并没有说这是错的。我只是觉得在标题的语境下有些令人困惑。 - smartcaveman

我不想挑剔，但我不明白为什么这个答案会得到那么多赞。第一部分与提供的警告没有任何关系，第二部分也没有提供解决方案... - Daniel Hilgarth

@Daniel：原帖问“从using块内部返回是否可以”，以及“是否应该重写代码将返回移动到using块外部”。我正在按照问题所问的回答，并且推测出问题的真实根本原因（因为原帖没有提供足够具体的细节）。你自己的回答提供了解决（假定的）真实问题的方法，如果你认为值得的话，我很愿意在这里复制类似的答案。 - LukeH

Answer 2

您的重写不会解决CA2000警告，因为问题不在 Tracer 对象上，而是在 MyResponse 对象上。
文档指出：

  

以下是一些使用语句不足以保护IDisposable对象并可能导致CA2000发生的情况。
  返回一个可处理对象需要在using块外部构造try / finally块。

要修复警告而不会干扰异常的调用堆栈（<- 点击链接），请使用此代码：

public MyResponse MyMethod(string arg)
{
   MyResponse tmpResponse = null;
   MyResponse response = null;
   try
   {
       tmpResponse = new MyResponse();

       using (Tracer myTracer = new Tracer(Constants.TraceLog))
       {
           // Some code
       }

       response = tmpResponse;
       tmpResponse = null;
    }
    finally
    {
        if(tmpResponse != null)
            tmpResponse .Dispose();
    }
    return response;
}

为什么？请参阅链接文档中的示例。

- Daniel Hilgarth

---

我认为try/catch可能更可取，因为它不需要你创建一个临时变量——在这里使用try/catch可能比问题本身更糟糕。 - Jon

@Jon：我发布的代码是一个具体的例子。我引用了我的初始分析基于的文档部分和我在答案中提到的示例也在文档页面上。它是该页面上唯一具有C#代码（带有SerialPort的代码）的示例。那个示例中没有“using”，因为它与警告完全无关。 - Daniel Hilgarth

@Jon：好的讨论，顺便说一下！ :-) - Daniel Hilgarth

@Jon，@Daniel：当然，你们是对的。我忽略了那个。虽然我可能会使用某种成功标志，并在finally中检查它，而不是交换response和tmpResponse。 - LukeH

@DanielHilgarth：你能举一个具体的例子，说明为什么使用try/catch解决方案（不需要额外变量）而不是try/finally可能会导致问题吗？请参见我的答案，了解我具体的意思。 - Jon

Answer 3

警告可能与MyResponse有关，它是IDisposable。

为什么会出现这个警告？

如果构造了一个MyResponse对象，但方法中后续的代码导致抛出异常，则所有对该对象的引用都将丢失（我们只有一个引用，并且未能返回它）。这意味着无法在对象上调用Dispose，我们将依赖于类终结器来清理任何资源。

这有影响吗？

一般而言，只有以下情况才会有影响：

IDisposable封装了其他部分或另一个进程“很快”可能需要的资源

在方法返回之前抛出异常，以触发“问题”

该资源没有被终结器及时释放，或者由于某种原因终结器从未运行，但应用程序没有崩溃

所以，实际上不应该有影响。

如何解决？

public MyResponse MyMethod(string arg)
{
    MyResponse abc = null;
    try {
        abc = new MyResponse();
        using (Tracer myTracer = new Tracer(Constants.TraceLog))
        {
            // Some code
           return abc;
        }
    }
    catch {
        if (abc != null) {
            abc.Dispose();
        }

        throw;
    }
}

这将确保如果控制通过异常退出该方法，则abc要么为null，要么已被正确处理。

更新

事实证明，当使用这种处理方式时，从MyMethod内部显式抛出的异常将被重新抛出，并且第一个堆栈帧的行号会变异以指向throw;语句。

实际上，这意味着如果在MyResponse中有多个throw语句，并且它们抛出相同类型的异常并具有相同的消息，则在捕获异常时您将无法准确确定哪个throw语句是负责的。

在我看来，这是一个纯粹学术上的问题，但我为了完整性而提到它。

- Jon

---

我总是会处理IDisposable对象。甚至一个IDisposable类都不能保证有一个finalizer。 - TrueWill

@TrueWill: 那么你会禁止任何返回 IDisposable 的方法吗？这样做不是有些过头了吗？这是那种策略的一个受害者：http://msdn.microsoft.com/en-us/library/b9skfh7s.aspx。确实应该处理可释放对象，但只有在使用完之后才能这样做。 - Jon

我不会禁止那个。方法可以返回一个IDisposable，但每个可处理的实例必须有一个所有者（即负责处理它的东西）。方法契约（可能是XML注释）应该指定调用方是否负责处理返回的实例。 - TrueWill

顺便说一句：您可以完全跳过“abc=null”赋值和“if(abc!=null)”检查，只需在try块之前放置“abc = new MyResponse();”即可，不需要任何更改（既不会丢失异常也不会丢失可释放实例）。 - springy76

Answer 4

“不太重要。但与@Aliostad相反，我认为版本2更好，因为return在using块之外，这样写更符合规范。

我的理由如下：

using块表示“打开”和“关闭”的事物。这是一种廉价交易。关闭using块表示我们已经完成了工作，现在可以安全地继续进行其他操作，例如return。”

- Daren Thomas

Answer 5

这个警告可能与“单一出口点”原则有关。在这里讨论了这个问题：http://c2.com/cgi/wiki?SingleFunctionExitPoint

- kek