Wait for messages in IMAP Gmail mailbox

I used the idle command to wait for incoming messages in my Gmail mailbox. The protocol I am using is IMAP.

My concern is as follows:

While the below code works, Gmail has a tendency to try to interrupt the connection. Therefore, I found some solutions online including sending a periodic NOOP command and catching the FolderClosedException exception and handling it by reopening the folder and continue listening idly.

This is what I came up with below and would like any feedback:

```
import java.util.Properties;

import javax.mail.Folder;
import javax.mail.FolderClosedException;
import javax.mail.MessagingException;
import javax.mail.Session;
import javax.mail.Store;
import javax.mail.event.MessageCountEvent;
import javax.mail.event.MessageCountListener;

import com.sun.mail.iap.ProtocolException;
import com.sun.mail.imap.IMAPFolder;
import com.sun.mail.imap.protocol.IMAPProtocol;

public class ImapReconnect {

private IMAPFolder imapFolder;
private IMAPFolder processedFolder;
private IMAPFolder invalidFolder;
private static final long KEEP_ALIVE_FREQ = 1000;
private static final String IRIDIUM_MAILBOX_PROCESSED="Processed";
private static final String IRIDIUM_MAILBOX_INVALID="Invalid";

private void startService(){
try {
setup();
} catch( MessagingException e) {
System.out.println("Error configuring imap server:");
System.out.println(e.toString());
System.exit(1);
}

Thread keepAlive = new Thread(new Runnable(){
public void run() {
keepAliveRunner();
}
});
keepAlive.start();

imapFolder.addMessageCountListener(new MessageCountListener(){

public void messagesAdded(MessageCountEvent arg0) {
System.out.println("New message was added.");
}

@Override
public void messagesRemoved(MessageCountEvent arg0) {

}

});

Threads:

Threads are hard. It's difficult to do it right and very very easy to mess up. One of the things you shouldn't do are so called Busy Waits.

These busy waits is calls to Thread.sleep() with values other than \$0\$ or \$1\$. You got one with the value \$1000\$

This is a big code-smell and I already got an idea why you did it that way.

Periodically doing stuff

Your design is somewhat off. Usually the checking of mailbox is accomplished a little differently than keeping the IMAP connection open.

Instead you once synchronize your folders and then have a scheduler redo that in the background again and again.

The standard way to do this seems to be (no experience on my side) to authenticate every request to the SMTP-Server.

You can accomplish this by using ScheduledExecutorService interface. There is a method scheduleAtFixedRate() provided. I suggest you use that instead of going for Task. Your code would then change from:

//... some code
    Thread keepAlive = new Thread(new Runnable(){
        public void run() {
            keepAliveRunner();
        }
    });
    keepAlive.start();
    //... other code ;)
}

public void keepAliveRunner(){
    while (!Thread.interrupted()) {
        try {
            // sleep for 5 minutes
            Thread.sleep(KEEP_ALIVE_FREQ);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        try {
            imapFolder.doCommand(new IMAPFolder.ProtocolCommand() {
                public Object doCommand(IMAPProtocol p)
                        throws ProtocolException {
                    p.simpleCommand("NOOP", null);
                    return null;
                }
            });
        } catch (MessagingException e) {
            e.printStackTrace();
        }
    }
}

to:

//... some code
    ScheduledExecutorService keepAlive = Executors.newScheduledThreadPool(1);
    Runnable toKeepAlive = new Runnable() {
        public void run() {
            keepAliveRunner();
        }
    };
    keepAlive.scheduleAtFixedRate(toKeepAlive,
          KEEP_ALIVE_FREQ, 
          KEEP_ALIVE_FREQ, 
          TimeUnit.MILLISECONDS); 
    /* you could use seconds / minutes instead, but then
    You'd need to change KEEP_ALIVE_FREQ ;) */
    //... other code ;)
}

public void keepAliveRunner() {
   try {
       imapFolder.doCommand(new IMAPFolder.ProtocolCommand() {
           public Object doCommand(IMAPProtocol p)
                   throws ProtocolException {
               p.simpleCommand("NOOP", null);
               return null;
           }
       });
    } catch (MessagingException e) {
      e.printStackTrace();
    }
}

This should accimplish the same, but doesn't block a whole thread with a busy wait (which isn't nice for your CPU). Also this largely reduces the complexity of your keepAliveRunner() method, which I would rename to runKeepAliveRequest() now.

This does not incorporate the changes I suggested earlier. I have no idea how the rest of your code works, so I just leave it like that and hope you can use it ;)

StackExchange Code Review Q#56403, answer score: 3