CODEWITHSUNDEEP
c#itextcode-analysis
Patrick Peters
Patrick Peters

Reputation: 9578

I can't get code analysis rule CA2202 fixed

I have a function (see code snippet below).

I have Code Analysis enabled, and I get a CA2202 rule violation.

(edit: I added the close on the pdfStamper otherwise the PDF will be corrupted)

CA2202: Do not dispose objects multiple times

A method implementation contains code paths that could cause multiple calls to IDisposable.Dispose or a Dispose equivalent, such as a Close() method on some types, on the same object.

In the CA2202 MSDN page(here), the proposed fix doesn't work.

How can the code be rewritten without having to suppress this violation ?

private byte[] DoGenerateFinishedGamePdf(int gameSessionLogId)
{
   var finishedGameCertificatePdfFile = httpServerUtilityWrapper.MapPath(ConfigurationManager.AppSettings["FinishedGameCertificateFile"]);

   var pdfReader = new PdfReader(finishedGameCertificatePdfFile); // note that PdfReader is not IDisposeable

   using (MemoryStream memoryStream = new MemoryStream())
   using (PdfStamper pdfStamper = new PdfStamper(pdfReader, memoryStream))
   {
      var fields = pdfStamper.AcroFields;
      fields.SetField("CityName", "It works!");

      pdfReader.Close();

      pdfStamper.FormFlattening = true;
      pdfStamper.FreeTextFlattening = true;
      pdfStamper.Close();

      return memoryStream.ToArray();
   }
}

Upvotes: 4

Views: 687

Answers (2)

Nicole Calinoiu
Nicole Calinoiu

Reputation: 21002

This is happening because the PdfStamper is disposing the stream even though it shouldn't. It didn't create it and doesn't own it, so it has no business disposing it.

Your code did create the stream and it does own it, so it's quite natural that it should dispose it. If the PdfStamper weren't disposing the stream inappropriately, all would be fine with your nested usings.

Your first move should probably be to file a bug report/feature request that the PdfStamper disposition of the stream be removed or at least made avoidable. Once you've done that, you can pretty safely suppress the CA2202 violation since double disposition of a MemoryStream will have no deleterious consequences.

BTW, PdfStamper.Dispose() call PdfStamper.Close() (at least in version 5.4.0), so you should be able to remove the PdfStamper.Close() call.

Upvotes: 1

Rich O'Kelly
Rich O'Kelly

Reputation: 41767

Ah, everyone's favourite warning! In this instance MemoryStream.Dispose is idempotent (the current implementation does nothing) so it's not really a problem, however the 'fix' is as follows:

MemoryStream memoryStream = null;
try
{
  memoryStream = new MemoryStream();
  using (PdfStamper pdfStamper = new PdfStamper(pdfReader, memoryStream))
  {
    memoryStream = null;

    var fields = pdfStamper.AcroFields;
    fields.SetField("CityName", "It works!");

    pdfReader.Close();

    pdfStamper.FormFlattening = true;
    pdfStamper.FreeTextFlattening = true;
    pdfStamper.Close();

    return memoryStream.ToArray();
  }
}
finally
{
  if (memoryStream != null) memoryStream.Dispose();
}

Since the PdfStamper 'owns' the MemoryStream it will dispose of it when PdfStamper.Dispose is called, so we only need to call Dispose on the MemoryStream if we do not Dispose of the PdfStamper, which can only happen if the construction of the PdfStamper fails.

Upvotes: 2

Related Questions