Problem
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) {
}
});
while (!Thread.interrupted()) {
try {
imapFolder.idle();
} catch (FolderClosedException e) {
System.out.println("The remote server closed the IMAP folder, we're going to try reconnecting.");
startService();
} catch (MessagingException e) {
System.out.println("Now closing imap mailbox, due to unhandlable exception: ");
System.out.println(e.toString());
break;
}
}
if (keepAlive.isAlive()) {
keepAlive.interrupt();
}
try {
imapFolder.close(false);
processedFolder.close(false);
invalidFolder.close(false);
} catch (MessagingException e) {
System.out.println("Error closing all the folders:");
System.out.println(e.toString());
}
}
private void setup() throws MessagingException{
Properties props = new Properties();
props.setProperty("mail.store.protocol", "imaps");
Session session = Session.getInstance(props, null);
Store store = session.getStore();
store.connect("imap.googlemail.com", 993,"myemail@gmail.com","password");
imapFolder = (IMAPFolder) store.getFolder("INBOX");
processedFolder = (IMAPFolder) imapFolder.getFolder(IRIDIUM_MAILBOX_PROCESSED);
if(!processedFolder.exists())
processedFolder.create(Folder.HOLDS_MESSAGES);
invalidFolder = (IMAPFolder) imapFolder.getFolder(IRIDIUM_MAILBOX_INVALID);
if(!invalidFolder.exists())
invalidFolder.create(Folder.HOLDS_MESSAGES);
imapFolder.open(Folder.READ_WRITE);
}
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();
}
}
}
public static void main(String[] args) {
ImapReconnect reconnect = new ImapReconnect();
reconnect.startService();
}
}
Solution
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 😉