0

My main question is if I pass object to a function that are using a MemoryStream how to I go about clearing the memorystream? See my code below.

In more detail: I'm trying to send multiple emails while creating PDFs on the fly using iTextSharper. I want to make sure that I'm not doing something wrong with the MemoryStream here. I can't use a using statement because when I do my EmailPDF function says the connections are closed, also if I don't use a new MemoryStream for each PDF creation it overrides the previously created one.

I'm adding the pdfs to an attachment and then adding the attachments to a list. I then use that list to add the attachment to a mailmessage. I then send the message.

        private void EmailPDF(List<String> lstFields)
    {            
        MailMessage mm = new MailMessage("fromemailaddress", "toemailaddress")
        {
            Subject = "Test Email",
            IsBodyHtml = true,
            Body = "Testing email"
        };

        SmtpClient smtp = new SmtpClient
        {
            Host = "xxx.xxx.xxx.xxx"
        };

        List<System.Net.Mail.Attachment> attachments = FillAttachmentList(lstFields);

        foreach (System.Net.Mail.Attachment attach in attachments)
        {
            mm.Attachments.Add(attach);
        }

        smtp.Send(mm);
        attachments.Clear();            
    }


        private List<System.Net.Mail.Attachment> FillAttachmentList(List<String> lstFields)
    {
        List<System.Net.Mail.Attachment> attachments = new List<System.Net.Mail.Attachment>();

        foreach (String strField in lstFields)
        {                
            MemoryStream output = new MemoryStream();
            Document doc = new Document(PageSize.LETTER, 25, 25, 25, 25);

            try
            {
                String text = System.IO.File.ReadAllText(@"C:\PDFDirectory\" + strField + ".html");
                StringReader html = new StringReader(text);

                PdfWriter wri = PdfWriter.GetInstance(doc, output);
                doc.Open();

                XMLWorkerHelper.GetInstance().ParseXHtml(wri, doc, html);

                wri.CloseStream = false;
                doc.Close();

                attachments.Add(new Attachment(output, strField + ".pdf"));
                output.Position = 0;
            }
            catch (Exception ex)
            {
            }
        }

        return attachments;
    }
J Hunt
  • 733
  • 1
  • 10
  • 21
  • 1
    If you set `PdfWriter`'s `CloseStream` property to `false` you should be able to use a using statement. [Like the example here](http://kuujinbo.info/iTextSharp/pdfSendMail.aspx). – kuujinbo Jun 16 '15 at 14:24

4 Answers4

2

One option is to add finally after the catch and use Close() or Dispose(). When they are called on a MemoryStream, they serve to do two things:

Mark the object disposed so that future accidental usage of the object will throw an exception. Possibly release references to managed objects, which can make the GC's job a bit easier depending on the GC implementation. (On today's GC algorithms it makes no real difference, so this is a point for an academic discussion and has no significant real-world impact.)

MemoryStream does not have any unmanaged resources to dispose, so you don't technically have to dispose of it. The effect of not disposing a MemoryStream is roughly the same thing as dropping a reference to a byte[] (the GC will clean both up the same way).

Which one do I call? Is it necessary to call both? The Dispose() method of streams delegate directly to the Close() method2, so both do exactly the same thing. And you can skip it if you want. The GC will clean it after all.

  • You don't want to close or dispose of the stream there for the reasons in the question - the stream is used by the `Attachment` and won't be read until the message is prepared for sending. The rest is correct though - I'd suggest 'do nothing'. – Charles Mager Jun 15 '15 at 15:57
  • That's what I said and I pointed to the fact that MemoryStream doesn't have unmanaged resources and it wont bother him. – Stoyan Uzunov Jun 15 '15 at 21:13
2

create a class

public class MyAttachment {
    MemoryStream output {get; set;}
    System.Net.Mail.Attachment {get; set;}
}

rewrite your FillAttachmentList(lstFields); method to provide a List<MyAttachment>;

instead of attachments.Clear(); write:

foreach (MyAttachment attach in attachments)
{
    if ( attach.output != null) {
        attach.output.Close();
    }
}
attachments.Clear();

In fact you should implement a MyAttachmentCollection : IEnumerable<MyAttachment>, IDisposable.

tschmit007
  • 7,559
  • 2
  • 35
  • 43
  • If you [follow it through](http://referencesource.microsoft.com/#System/net/System/Net/mail/MailMessage.cs,149b686c70a3e745,references) then this would actually be achieved by just disposing of the `MailMessage` – Charles Mager Jun 15 '15 at 16:21
1

In your context, like @Stoyan said, it makes no much difference, if you want you can force GC to swipe the streams by using GC.Collect().

Coastpear
  • 374
  • 2
  • 15
  • `GC.Collect()` is generally considered as bad practice. See http://stackoverflow.com/questions/118633/whats-so-wrong-about-using-gc-collect – hdoghmen Jun 15 '15 at 21:17
1

Your MemoryStream has no reference to unmanaged resources, so you're probably fine not to close or dispose of it. A search will find other questions that address this point.

However, MailMessage and all its component parts implement IDisposable. If you follow through the reference source you'll see that your stream in your attachment would be closed by disposing of your MailMessage.

So, to be a good citizen you should either call mm.Dispose() directly after sending the message or wrap your use of the MailMessage in a using statement.

Community
  • 1
  • 1
Charles Mager
  • 25,735
  • 2
  • 35
  • 45