Prev: toolstrip - mouse event for disabled items?
Next: Calling Internal Method using Reflection
From: za on 7 Jun 2010 06:11

"Peter Duniho" <NpOeStPeAdM(a)NnOwSlPiAnMk.com> wrote in message
news:Qf2dnQO8cpBmKZHRnZ2dnUVZ_tWdnZ2d(a)posted.palinacquisition...
> za wrote:
>> using System;
>> using System.Configuration;
>>
>> namespace CA2000
>> {
>> internal class Settings1 : ApplicationSettingsBase, IDisposable
>> {
>> private static Settings1 defaultInstance =
>> ((Settings1)(ApplicationSettingsBase.Synchronized(new Settings1())));
>>
>> public static Settings1 Default
>> {
>> get
>> {
>> return defaultInstance;
>> }
>> }
>>
>> public void Dispose()
>> {
>>
>> }
>> }
>> }
>>
>> Maybe this is the simplest sample to reproduce the CA2000 error.
>
> Well, actually.the above example doesn't need the Default property in
> order to reproduce the warning. That said, it's a good example, and
> provides plenty to work with in terms of answering the question.
>
> Unfortunately, I'm not there's a good _answer_. :)
>
> As you're probably already aware, the bottom line issue here is that an
> object is allocated, and then as far as the code analysis can tell, is not
> disposed of before you no longer have a reference to it.
>
> Of course, the object returned by the Synchronized() method in fact refers
> to the original object (indeed, _is_ the original object), and so the
> reference still exists, and so doesn't need to be disposed (and in fact,
> cannot be). But there's no direct way for the code analyzer to know that.
> It can't make assumptions about what the Synchronized() method does, and
> it doesn't do any sort of called-method inspection to make its
> determinations.
>
> As far as potential resolutions go, here is a version of the code that
> doesn't cause the warning:
>
> using System;
> using System.Configuration;
>
> namespace CA2000
> {
> internal class Settings1 : ApplicationSettingsBase, IDisposable
> {
> private static Settings1 defaultInstance;
>
> static Settings1()
> {
> defaultInstance = new Settings1();
>
> defaultInstance =
> (Settings1)ApplicationSettingsBase.Synchronized(defaultInstance);
> }
>
> public void Dispose()
> {
>
> }
> }
> }
>
> Note that by writing the static constructor explicitly, _and_ by
> assigning the original Settings1() object to your field before passing
> it to the Synchronized() method, that's enough to convince code analysis
> that you've let the object reference escape the constructor, and thus it
> does not need to be disposed of before the constructor returns.
>
> I find this resolution unfortunate in two ways:
>
> . First, it would be nice if there were some way to explain to the
> analyzer that in this case, passing the object reference to the
> Synchronized() method does in fact preserve a reference to the object
> after the constructor returns.
>
> . Second, it concerns me that the code analysis doesn't pick up on the
> fact that the "defaultInstance" field is being overwritten, which absent
> any specific knowledge of the Synchronized() method (which clearly the
> analyzer does not have) _should_ in fact mean that the original object
> reference has been lost before the object has been disposed.
>
> I find this second issue especially ironic, because it appears that when
> the variable is a local variable (i.e. if you try to move the
> initialization into a helper method with a try/catch to deal with
> disposing the original object on error), the analyzer _does_ in fact pick
> up on the fact that it's been overwritten, and so returning the new value
> does not satisfy the analyzer with respect to the original value.
>
> I don't really see a good way around the first issue, other than just
> suppressing the warning (which is a blunt way to give the analyzer that
> information). The second issue concerns me because it's possible someone
> at Microsoft will eventually decide it's a bug that the analyzer doesn't
> figure that out and will fix the bug, and then my proposed work-around
> isn't going to help.
>
> I think given all the above, personally I'd just suppress the warning and
> be done with it. The fact is, it's a situation that given the kind of
> analysis used for that rule, I think really cannot be detected as safe by
> the analyzer, so you're left helping it out by suppressing the warning
> explicitly.
>
> Pete

Hi Pete,

Thank you for your detailled explanation and also for the third work around!
Personally I think that it is a bug in the analyzer, but i am not sure about
it :-)

I've also tried this code, but it gives a CA2000 too.

using System;
using System.Configuration;

namespace CA2000
{
internal class Settings1 : ApplicationSettingsBase, IDisposable
{
private static Settings1 defaultInstance; // =
((Settings1)(ApplicationSettingsBase.Synchronized(new Settings1())));

public static Settings1 Default
{
get
{
if (defaultInstance == null)
{
Settings1 temp = null;
try
{
temp = new Settings1();
temp = (Settings1)Settings1.Synchronized(temp);
defaultInstance = temp;
temp = null;
}
finally
{
if (temp != null)
{
temp.Dispose();
}
}
}

return defaultInstance;
}
}

public void Dispose()
{

}
}
}

Regards,

Fred


From: Peter Duniho on 7 Jun 2010 11:31
za wrote:
> [...]
> Thank you for your detailled explanation and also for the third work around!
> Personally I think that it is a bug in the analyzer, but i am not sure about
> it :-)

I would not call it a bug. There really is no practical way for the
analyzer to know the difference between a method that returns an
unrelated object when you pass in an object, and a method that returns a
related (or even identical) object to the one passed in.

> I've also tried this code, but it gives a CA2000 too.
>
> [...]
> try
> {
> temp = new Settings1();
> temp = (Settings1)Settings1.Synchronized(temp);
> defaultInstance = temp;
> temp = null;
> } [...]

Right. But because the analyzer _can_ tell that the local variable
"temp" is set to null, thus losing the reference, before being disposed
the warning is still emitted.

The basic logic here is similar to that used for "definite assignment".
That is, the analyzer is looking for "definite 'return'", where
'return' simply means that the object reference gets _out_ of the
method. Calling a method can't satisfy that requirement, because while
the method _could_ retain the reference somehow, it doesn't _have_ to.

Reasoning that a false positive is better than a false negative, the
analyzer always warns unless it can be _sure_ that the reference has
left the method.

In the end, any work-around for the issue is likely to result in code
that is much more awkward than would normally be needed. Unlike the
definite assignment scenario, where a single assignment is generally
sufficient, the work-arounds for CA2000 false-positives all involve
tailoring the code to suit the analyzer's particular mechanisms.

That's why, in spite of the presence of work-arounds, it would be my
inclination to just write the code the way you feel is most appropriate,
and then put a suppression in.

Pete
From: za on 7 Jun 2010 14:05

"Peter Duniho" <NpOeStPeAdM(a)NnOwSlPiAnMk.com> wrote in message
news:mv-dnQHqvO9IjZDRnZ2dnUVZ_uidnZ2d(a)posted.palinacquisition...
> za wrote:
>> [...]
>> Thank you for your detailled explanation and also for the third work
>> around! Personally I think that it is a bug in the analyzer, but i am not
>> sure about it :-)
>
> I would not call it a bug. There really is no practical way for the
> analyzer to know the difference between a method that returns an unrelated
> object when you pass in an object, and a method that returns a related (or
> even identical) object to the one passed in.
>
>> I've also tried this code, but it gives a CA2000 too.
>>
>> [...]
>> try
>> {
>> temp = new Settings1();
>> temp = (Settings1)Settings1.Synchronized(temp);
>> defaultInstance = temp;
>> temp = null;
>> } [...]
>
> Right. But because the analyzer _can_ tell that the local variable "temp"
> is set to null, thus losing the reference, before being disposed the
> warning is still emitted.
>
> The basic logic here is similar to that used for "definite assignment".
> That is, the analyzer is looking for "definite 'return'", where 'return'
> simply means that the object reference gets _out_ of the method. Calling
> a method can't satisfy that requirement, because while the method _could_
> retain the reference somehow, it doesn't _have_ to.
>
> Reasoning that a false positive is better than a false negative, the
> analyzer always warns unless it can be _sure_ that the reference has left
> the method.
>
> In the end, any work-around for the issue is likely to result in code that
> is much more awkward than would normally be needed. Unlike the definite
> assignment scenario, where a single assignment is generally sufficient,
> the work-arounds for CA2000 false-positives all involve tailoring the code
> to suit the analyzer's particular mechanisms.
>
> That's why, in spite of the presence of work-arounds, it would be my
> inclination to just write the code the way you feel is most appropriate,
> and then put a suppression in.
>
> Pete

Hi Pete,

Thank you for your time and answers. I will supress the warning(s)

Regards,
Fred


First  |  Prev  | 
Pages: 1 2
Prev: toolstrip - mouse event for disabled items?
Next: Calling Internal Method using Reflection