skip to Main Content

I suspect that I’m using wrong Apache Common Email library to send email with attachments.
Sometimes I got “Too many open files” error and if I check the opened files with lsof I see that the attachment files are opened multiple times:

image1.png
image2.png
image1.png

So I suspect that I am not releasing files correctly (or not closing some resources). It could be also that the software works but if the external SMTP server failed (connection exception), my code doesn’t release some resources.

The source:

/* Async Thread to send email */
Thread t = new Thread(){
@Override
public void run(){
      try{
         task(request,idNewsletter);
      }
      catch (Exception e){
         log.error("Error " + e);
      }
}
};
t.start();
[...]

the task method:

    ExecutorService executor = Executors.newFixedThreadPool(10);
    [...]

    List<Future<Integer>> list = new ArrayList<Future<Integer>>();
    for (int i = 0; i < dests.length; i++) {        
        HtmlEmail htmlEmail = getHtmlMail([...]);

        ArrayList<InternetAddress> dest = new ArrayList<InternetAddress>();
        InternetAddress add = new InternetAddress();
        add.setAddress(dests[i]);
        dest.add(add);

        htmlEmail.setBcc(dest);         
        htmlEmail.setMailSession(getEmailSession());            
        htmlEmail.buildMimeMessage();

        Callable<Integer> worker = new EmailService(htmlEmail,i);        
        Future<Integer> future = executor.submit(worker);            
        list.add(future);

    }

    try {
        executor.shutdown();
        executor.awaitTermination(28800, TimeUnit.SECONDS);
    }
    catch (InterruptedException e) {
       log.error("Error=" + e);
    }
    finally {
        if (!executor.isTerminated()) {
            log.error("Task cancelled");
        }
        executor.shutdownNow();
    }

the getHtmlEmail method

private static URL image;
static {
     image= (new File("somepath").toURI().toURL());
 }


private HtmlEmail getHtmlMail([...]){

      HtmlEmail htmlEmail = new HtmlEmail();

        htmlEmail.setCharset("UTF-8");
        htmlEmail.setSubject("someSubject");
        htmlEmail.setFrom(from);
        htmlEmail.setSentDate(new java.util.Date());
        [...]
        String cidImage= htmlEmail.embed(image, "Email image");
        text= text.replace("cid:image", "cid:" + cidImage);

        [...] /* Other attachments */

        htmlEmail.setHtmlMsg(text); 
        return htmlEmail;

}

the getEmailSession method

private Session getEmailSession() throws Exception{  
        if (mailSession != null)
            return mailSession;
        else {
            InitialContext ic = new InitialContext();
            mailSession = (Session) ic.lookup("someJNDI");

            mailSession.getProperties().put("mail.smtp.connectiontimeout", 1000);
            mailSession.getProperties().put("mail.smtp.timeout", 5000);

            return mailSession;
        }

   }

and the method which sends the email

@Override
public Integer call() throws Exception {    
    try{                                                        
           htmlEmail.sendMimeMessage();    
       }
    catch(Exception e){  
           log.error("Error=" + e);
           htmlEmail=null;
           return new Integer(0);
       }                         
      return new Integer(1);
}  

Any hints?

Thanks in advance
Andrea

2

Answers


  1. Chosen as BEST ANSWER

    Update: even if I rewrite fields as non static the problem persists. I suppose it is a problem (bug?) with this version of library (Apache Common Email 1.5)


  2. I believe this is the problem:

    static {
         image= (new File("somepath").toURI().toURL());
     }
    

    It’s static. It’s kind of a concurrency problem you’re having.
    It’s always there.
    Here’s what I think is happening. Let’s say this is inside a class called Url and you have a non static getUrl() which returns this.
    If you do this:

    Url url1 = new Url();
    Url url2 = new Url();
    url1.getUrl();
    url2.getUrl();
    

    Then they’re each supposed to return a different instance of the url. However, you’re initializing the url property as static, so what’s really going on is all instances of Url only have one url. The url property is not owned by each object, but only by the class itself.
    So when you try to call it, they’re all trying to access the file again and again because it was never “created” nor initialized per instance in the first place. Think of it as something that’s always there, because the Class itself owns it, and not the instances, thus giving you the Too many open files error.

    So I’d suggest you remove it from being static. Maybe you can do something like this:

    class Url{
        private final URL url;
        public Url() {
            url= (new File("somepath").toURI().toURL());
        }
    }
    

    And then just use that url property wherever you want.

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search