How to use IDisposable in a lambda expression?

Question

I'm in the process of doing some code analysis on a project and implementing the suggestions that make sense. One suggestion is to do the following:

CA2000 : Microsoft.Reliability : In method 'Service.ParseConfigurationFile()', call System.IDisposable.Dispose on object 'new SecureString()' before all references to it are out of scope

The offending line is as follows:

 Password = me.Password.Aggregate(new SecureString(), (secureString, c) => { secureString.AppendChar(c); return secureString; })

Any ideas on how to do this properly? I have replaced the above with the line below but I don't think it's correct as it still causes the code analysis message to come up:

Password = me.Password.Aggregate(new SecureString(), (secureString, c) => { using (secureString) {secureString.AppendChar(c); return secureString;} })

EDIT: As per comment from @Jon below objectInstance is an instance of a custom class called MailboxElement (me). It's going through multiple custom sections in a config file which looks as follows:

foreach (MailboxElement me in mailboxesSection.Mailboxes)
{
      MailboxInformation mailboxInformation = new MailboxInformation
      {
                    ExchangeServerWebServiceUrl = me.ExchangeServerWebServiceUrl,
                    MailboxFriendlyName = me.FriendlyName,
                    UserName = me.UserName,
                    Password = me.Password.Aggregate(new SecureString(), (secureString, c) => { secureString.AppendChar(c); return secureString; }),
                    MailboxToAccess = me.MailboxToAccess
      };

      // Do stuff with mailboxInformation here
}

MailboxElement is a sealed class that implements ConfigurationElement that has all the properties noted above.

MailboxInformation is defined as follows:

public class MailboxInformation
{
   public string MailboxFriendlyName { get; set; }
   public string UserName { get; set; }
   public SecureString Password { get; set; }
   public string ExchangeServerWebServiceUrl { get; set; }
   public string MailboxToAccess { get; set; }
   public string InboxFolderId { get; set; }
   public string SentItemsFolderId { get; set; }
   public bool MailboxSettingsDiscovered { get; set; }
}

I hope this makes things clearer...

Here are some good inputs: http://blog.linqexchange.com/index.php/how-to-use-idisposable-with-linq/ — Jocke, Jul 03 '12 at 10:11
It would be helpful if you said what `objectInstance.Password` is and what this code is intended to do. — Jon, Jul 03 '12 at 10:11
Could it be whatever you do with `Password` after this line of code? The LINQ itself shouldn't result in any lost `SecureStrings`, so either the code analyser isn't intelligent enough to realise this or you're letting go of `Password`. — Rawling, Jul 03 '12 at 10:12
Is "Password" a field (maybe a property) of the current instance? — Maghis, Jul 03 '12 at 10:19
I put the following suppression in my code: ``[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000: Dispose objects before losing scope. This is a broken rule. See http://stackoverflow.com/q/2687398/228059 for more")]`` — noonand, Jul 03 '12 at 10:53

Rune FS · Answer 1 · 2012-07-03T11:49:40.753

3

You are returning an IDispoable from your Aggregation so you will need to dispose that or simply reassign to secureString

using (var secureString = new SecureString()) {
    secureString = objectInstance.Password.Aggregate((secureString, c) => { secureString.AppendChar(c); return secureString; }){
}

EDIT after question update

Since your MailboxInformation objects now get ownership of an IDisposable the MailboxInformation it self should be IDisposable and disposing the disposables it owns

so the implementation would be (similar to your comment)

public class MailboxInformation : IDisposeable
{
   //...
   public SecureString Password { get; set; }
   //...
   void IDisposable.Dispose() {
      this.Password.Dispose();
   }
}

edited Jul 03 '12 at 11:49

answered Jul 03 '12 at 10:29

Rune FS

21,497
7
62
96

Complicated by the fact it's part of an object initialiser. See expanded question – noonand Jul 03 '12 at 10:31
1

I implement IDisposable, and inside in the Dispose() method I call `this.Password.Dispose();` ? – noonand Jul 03 '12 at 11:29
@noonand yeah I've updated with a sample implementation. The warning might still persist due to the bug in the analyser but you will have fixed the current issue but since you have pushed the task to those that uses MailboxInformaiton you should get more warnings now. This however is a good thing and simply because the system can now identify the missing disposals (and some false positives) – Rune FS Jul 03 '12 at 11:49
Thanks for that, as you correctly surmised the error still occurs. This means that @Rajesh Subramanian's answer is correct i.e. the rule is broken. I appreciate the time you took to write this up. – noonand Jul 03 '12 at 12:12
1

@noonand but don't forget to look at those places where you instantiate a MailboxInformation object because you need to dispose those and beware that you could potentially with your design share an IDisposable between several object each taking ownership – Rune FS Jul 03 '12 at 13:00
1

@noonand further in the code you've showb so far (with or without the above suggestion) the warning is **correct** so unless you've fixed all the valid causes you want know if there are some invalid once in your case as well – Rune FS Jul 08 '12 at 18:48

Samuel Parkinson · Answer 2 · 2012-07-03T10:23:10.670

1

Try this format:

using (var secureString = new SecureString()) {
    Password = objectInstance.Password.Aggregate([...])
}

It is better to encase the whole thing in a using statment I believe.

edited Jul 03 '12 at 10:23

answered Jul 03 '12 at 10:10

Samuel Parkinson

2,992
1
27
38

+1 For correct answer. The `secureString` is used during the whole aggregation process, and therefore the `using` should be outside of it. Further more - using the `secureString` should be in the `using` statement, in addition to the aggregation. – SimpleVar Jul 03 '12 at 10:13
@RajeshSubramanian In this case, the warning can be ignored. Sam presents the right way to handle it. – SimpleVar Jul 03 '12 at 10:14
Yes.. That's Okay. Still not the solution to the problem – Rajesh Subramanian Jul 03 '12 at 10:17
@RajeshSubramanian I hadn't tested it I'm afraid. And I've always though that classes that inherit `IDisposable` should be contained in a `using` statement. Tricky to know when with a broken tool I guess :P – Samuel Parkinson Jul 03 '12 at 10:17
Complicated by the fact it's part of an object initialiser. See expanded question – noonand Jul 03 '12 at 10:49

score 1 · Accepted Answer · edited May 23 '17 at 11:48

1

By this link , it is a broken rule and no need to consider that one, Please ignore that rule.

CA2000 passing object reference to base constructor in C#

http://www.debugging.com/bug/24060

edited May 23 '17 at 11:48

Community

1
1

answered Jul 03 '12 at 10:12

Rajesh Subramanian

6,400
5
29
42

So bottom line is there's nothing I can do? Should I supress the error message using an attribute? – noonand Jul 03 '12 at 10:35
2

@noonand in your particular case the warning is correct. You are assigning the IDisposable to a member of a type that itself is not disposable therefor you have a reference to an IDisposable that will not be disposed when the last reference goes out of scope. Just because a warning can sometimes be ignored doesn't meen you should always ignore it. Use your analytic vit to see if it's an issue before ignoring – Rune FS Jul 03 '12 at 11:20

How to use IDisposable in a lambda expression?

3 Answers3