0

I'm developing an android app that receives and processes mail messages. The app must be connected to an IMAP server and keep the connection alive, so it can see and process new mail messages instantly (mails contains json data from a mail api server). The app have two modes, manual and live connection. Here is some of my code:

class Idler {
Thread th;
volatile Boolean isIdling=false;
boolean shouldsync=false;//we need to see if we have unseen mails
Object idleLock;
Handler handler=new Handler();
IMAPFolder inbox;
public boolean keppAliveConnection;//keep alive connection, or manual mode

//This thread should keep the idle connection alive, or in case it's set to manual mode (keppAliveConnection=false) get new mail.
Thread refreshThread;
synchronized void refresh()
{
    if(isIdling)//if already idling, just keep connection alive
    {
        refreshThread =new Thread(new Runnable() {
            @Override
            public void run() {
                try {
                    inbox.doCommand(new IMAPFolder.ProtocolCommand() {
                        @Override
                        public Object doCommand(IMAPProtocol protocol) throws ProtocolException {
                            //Why not noop?
                            //any call to IMAPFolder.doCommand() will trigger waitIfIdle, this
                            //issues a "DONE" command and waits for idle to return(ideally with a DONE server response).
                            // So... I think NOOP is unnecessary
                            //protocol.simpleCommand("NOOP",null); I'm not issuing noop due to what I said ^

                            //PD: if connection was broken, then server response will never arrive, and idle will keep running forever
                            //without triggering messagesAdded event any more :'( I see any other explanation to this phenomenon


                            return null;
                        }
                    });
                } catch (MessagingException e) {
                    e.printStackTrace();
                }
            }
        },"SyncThread");
        refreshThread.start();
    }
    else
    {
        getNewMail();//If manual mode keppAliveConnection=false) get the new mail
    }
}
public Idler()
{
    th=new Thread(new Runnable() {

        @SuppressWarnings("InfiniteLoopStatement")
        @Override
        public void run() {
            while (true)
            {
                try {
                    if(refreshThread !=null && refreshThread.isAlive())
                        refreshThread.interrupt();//if the refresher thread is active: interrupt. I thing this is not necessary at this point, but not shure
                    initIMAP();//initializes imap store
                    try {
                        shouldsync=connectIMAP()||shouldsync;//if was disconnected or ordered to sync: needs to sync
                    }
                    catch (Exception e)
                    {
                        Thread.sleep(5000);//if can't connect: wait some time and throw
                        throw e;
                    }
                    shouldsync=initInbox()||shouldsync;//if inbox was null or closed: needs to sync
                    if(shouldsync)//if needs to sync
                    {
                        getNewMail();//gets new unseen mail
                        shouldsync=false;//already refreshed, clear sync "flag"
                    }

                    while (keppAliveConnection) {//if sould keep idling "forever"
                        synchronized (idleLock){}//MessageCountListener may be doing some work... wait for it
                        isIdling = true; //set isIdling "flag"
                        handler.removeCallbacksAndMessages(null);//clears refresh scheduled tasks
                        handler.postDelayed(new Runnable() {
                            @Override
                            public void run() {
                                refresh();
                            }
                        },1200000);//Schedule a refresh in 20 minutes
                        inbox.idle();//start idling
                        if(refreshThread !=null && refreshThread.isAlive())
                            refreshThread.interrupt();//if the refresher thread is active: interrupt. I thing this is not necessary at this point, but not shure
                        handler.removeCallbacksAndMessages(null);//clears refresh scheduled tasks
                        isIdling=false;//clear isIdling "flag"
                        if(shouldsync)
                            break;//if ordered to sync... break. The loop will handle it upstairs.
                        synchronized (idleLock){}//MessageCountListener may be doing some work... wait for it

                    }
                }
                catch (Exception e) {
                    //if the refresher thread is active: interrupt
                    //Why interrupt? refresher thread may be waiting for idle to return after "DONE" command, but if folder was closed and throws
                    //a FolderClosedException, then it could wait forever...., so... interrupt.
                    if (refreshThread != null && refreshThread.isAlive())
                        refreshThread.interrupt();
                    handler.removeCallbacksAndMessages(null);//clears refresh scheduled tasks
                }
            }
        }
    },"IdlerThread");
    th.start();
}

private synchronized void getNewMail()
{
    shouldsync=false;
    long uid=getLastSeen();//get last unprocessed mail
    SearchTerm searchTerm=new UidTerm(uid,Long.MAX_VALUE);//search from las processed message to the las one.
    IMAPSearchOperation so=new IMAPSearchOperation(searchTerm);
    try {
        so.run();//search new messages
        final long[] is=so.uids();//get unprocessed messages count
        if (is.length > 0) {//if some...
            try {
                //there are new messages
                IMAPFetchMessagesOperation fop=new IMAPFetchMessagesOperation(is);
                fop.run();//fetch new messages
                if(fop.messages.length>0)
                {
                    //process fetched messages (internally sets the last seen uid value & delete some...)
                    processMessages(fop.messages);
                }
                inbox.expunge();//expunge deleted messages if any
            }
            catch (Exception e)
            {
                //Do something
            }
        }
        else
        {
            //Do something
        }
    }
    catch (Exception e)
    {
        //Do something
    }
}


private synchronized void initIMAP()
{
    if(store==null)
    {
        store=new IMAPStore(mailSession,new URLName("imap",p.IMAPServer,p.IMAPPort,null,p.IMAPUser,p.IMAPPassword));
    }
}

private boolean connectIMAP() throws MessagingException {
    try {
        store.connect(p.IMAPServer, p.IMAPPort, p.IMAPUser, p.IMAPPassword);
        return true;
    }
    catch (IllegalStateException e)
    {
        return false;
    }
}

//returns true if the folder was closed or null
private synchronized boolean initInbox() throws MessagingException {
    boolean retVal=false;
    if(inbox==null)
    {//if null, create. This is called after initializing store
        inbox = (IMAPFolder) store.getFolder("INBOX");
        inbox.addMessageCountListener(countListener);
        retVal=true;//was created
    }
    if(!inbox.isOpen())
    {
        inbox.open(Folder.READ_WRITE);
        retVal=true;//was oppened
    }
    return retVal;
}

private MessageCountListener countListener= new MessageCountAdapter() {
    @Override
    public void messagesAdded(MessageCountEvent ev) {
        synchronized (idleLock)
        {
            try {
                processMessages(ev.getMessages());//process the new messages, (internally sets the last seen uid value & delete some...)
                inbox.expunge();//expunge deleted messajes if any
            } catch (MessagingException e) {
                //Do something
            }

        }
    }
};

}

The problem is: Sometimes when the user is refreshing or the app auto-refreshes, in the Alive Connection mode, one or both of this conditions keeps my app from getting new messages. This is from the javamail source code.

1: The IdlerThread enters monitor state in:

//I don't know why sometimes it enters monitor state here.
private synchronized void throwClosedException(ConnectionException cex) 
        throws FolderClosedException, StoreClosedException {
// If it's the folder's protocol object, throw a FolderClosedException;
// otherwise, throw a StoreClosedException.
// If a command has failed because the connection is closed,
// the folder will have already been forced closed by the
// time we get here and our protocol object will have been
// released, so if we no longer have a protocol object we base
// this decision on whether we *think* the folder is open.
if ((protocol != null && cex.getProtocol() == protocol) ||
    (protocol == null && !reallyClosed))
        throw new FolderClosedException(this, cex.getMessage());
    else
        throw new StoreClosedException(store, cex.getMessage());
}

2: The "refresherThread" enters wait state in:

  void waitIfIdle() throws ProtocolException {
assert Thread.holdsLock(messageCacheLock);
while (idleState != RUNNING) {
    if (idleState == IDLE) {
    protocol.idleAbort();
    idleState = ABORTING;
    }
    try {
    // give up lock and wait to be not idle
    messageCacheLock.wait();//<-----This is the line is driving me crazy.
    } catch (InterruptedException ex) { }
}
}

As one of both of this threads "stops" running (wait & monitor state) my app is useless when reach this condition. In my country the mobile data network is very unstable, slow & expensive(GSM) So it must be failure resilient and take care about every transferred bit.

I guess the problem arises when the connection silently fails and the refresherThread starts to do its job. It issues a DONE command if idle is active, but, as the connection is gone, when idle tries to throw a FolderClosedException, one or both threads gets locked indefinitely.

So, my question is: Why is this situation arising and how to prevent it? How can I keep the idle loop securely running without getting locked?

I've tried a lot of things till exhaustion with no results.

Here are some threads I've read without getting a solution to my problem. In my country internet is EXTREMELY expensive too, so I can't research as much as I want, nor list all the urls I've visited looking for information.

JavaMail: Keeping IMAPFolder.idle() alive

JavaMail: Keeping IMAPFolder.idle() alive

Javamail : Proper way to issue idle() for IMAPFolder

Please, excuse my english. Any suggestion will be greatly appreciated. I've heard about this site strictness, so please be gentle, I'm new over here.

Raymond Arteaga
  • 4,355
  • 19
  • 35

1 Answers1

1

Be sure to set the timeout properties to make sure you don't hang waiting for a dead connection or server.

Instead of issuing a nop command directly, you should call Folder.isOpen or Folder.getMessageCount; they'll issue the nop command if needed.

If the folder is closed asynchronously (FolderClosedException), you'll need to restart the idle loop.

Bill Shannon
  • 29,579
  • 6
  • 38
  • 40
  • Indeed, I think that happens when it throws a FolderClosedException somewhere (too many mess to debug). What do you mean with restart the idle loop? – Raymond Arteaga Oct 09 '17 at 22:47
  • If the Folder is closed asynchronously, the Folder.idle call will fail. That might leave that thread in a tight loop since you're ignoring all exceptions. It would be better to catch that exception, reopen the folder (if that's what you intend), and then restart the idle loop. – Bill Shannon Oct 10 '17 at 03:06
  • The code is larger than the fragment I posted. It has all that stuff already implemented, but I wanted to keep it short. I guess I solved the problem, but the "supossed" explanation of what I think is happening don't fit in the allowed characters count. Is there some "correct" way to add a "bigger" comment? – Raymond Arteaga Oct 12 '17 at 04:00
  • Update the original post. – Bill Shannon Oct 12 '17 at 15:54
  • I've edited my question with a more explicative text. Please give a look at it. It's driving me crazy. @BillShannon – Raymond Arteaga Oct 13 '17 at 20:28
  • I don't see any setting of timeout properties in your code. Without that being set properly, connections can hang, causing the sorts of problems you're seeing. – Bill Shannon Oct 14 '17 at 00:12
  • I've read there is no way to set "idle timeout" in session properties, but, in case I set the socket timeout in 20 minutes, and the user refreshes the app in less than 20 minutes, how would that help? this "error" condition must yet be reached... mustn't? Ok.. it only will hang for less than 20 minutes... but still, that's not what i want. @Bill – Raymond Arteaga Oct 14 '17 at 13:57
  • If the connection is dead, you don't want to wait forever. You should probably set the timeout to less than 20 minutes. It's not the idle timeout, it's how long to wait for any response from the server. If the idle command hits this timeout, it will just retry. You might also need to set the write timeout. – Bill Shannon Oct 14 '17 at 18:00