2

Every time on visual studio 2015, when i run Code Analysis, there are some annoying warnings. All of them are in a methods like this:

here is my method:

public static JObject ReadJson(string file_path)
{
    try {
        JObject o1 = JObject.Parse(File.ReadAllText(file_path));
        using (StreamReader file = File.OpenText(file_path))
        {
            using (JsonTextReader reader = new JsonTextReader(file))
            {
                return (JObject)JToken.ReadFrom(reader);//the warning is here
            }
        }
    }
    catch
    {
        return default(JObject);
    }

}

so why this warning occur? How to solve it? And the most important is what my fault in this method it seems to me very perfect

Warning Description

Severity Code Description Project File Line Warning CA2202 : Microsoft.Usage : Object 'file' can be disposed more than once in method 'JsonHelper.ReadJson(string)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.

aug born
  • 248
  • 1
  • 3
  • 20

1 Answers1

3

MSDN:

Nested using statements (Using in Visual Basic) can cause violations of the CA2202 warning. If the IDisposable resource of the nested inner using statement contains the resource of the outer using statement, the Dispose method of the nested resource releases the contained resource. When this situation occurs, the Dispose method of the outer using statement attempts to dispose its resource for a second time.

Problem:

using (StreamReader file = File.OpenText(file_path))
{
    using (JsonTextReader reader = new JsonTextReader(file))
    {
        return (JObject)JToken.ReadFrom(reader);//the warning is here
    }   //"file" will be disposed here for first time when "reader" releases it
}   //and here it will be disposed for the second time and will throw "ObjectDisposedException"

Solution:

You need to do it like this(disposing the object in finally block when all goes right, or in catch block when an error occurs):

public static JObject ReadJson(string file_path)
{   
    StreamReader file = null;
    try {
        JObject o1 = JObject.Parse(File.ReadAllText(file_path));
        file = File.OpenText(file_path);
        using (JsonTextReader reader = new JsonTextReader(file))
        {
            return (JObject)JToken.ReadFrom(reader);
        }
    }
    catch
    {
        return default(JObject);
    }
    //dispose "file" when exiting the method
    finally
    {
        if(file != null)
            file.Dispose();
    }
}
Shaharyar
  • 12,254
  • 4
  • 46
  • 66
  • I think you need to remove the dispose from the catch, or you'll double-dispose here too... – Clockwork-Muse Dec 26 '15 at 09:14
  • @Clockwork-Muse Yup ! thanks for pointing. – Shaharyar Dec 26 '15 at 09:19
  • This answer misses the key part of the linked documentation. Since the `JsonTextReader` object calls `Dispose()` on the underlying `StreamReader` that it is passed when itself is `Dispose()`-ed, you should not call it yourself. The solution is to reset your reference to the `StreamReader` to null inside the `using (JsonTextReader...)` block so you don't re-`Dispose()` in your `finally`. What you have posted basically the expansion the compiler does for you when using `using` (albeit slightly different since you share the existing `try` block), and does not fix the CA warning. – clcto Aug 20 '21 at 13:32