Hi there I have an application that connects to n inboxes of users. Since I don't won't to poll new messages I am using the IMAP idle command and a message count listener to listen to new messages.
Since imap.idle() blocks the current thread I am creating a thread for each user which then is being blocked.
this.mailboxExecutor = Executors.newCachedThreadPool();
ConcurrentHashMap<Long, Future<?>> mailboxFutures =
new ConcurrentHashMap<>();
private void startMailboxListenerForUser(User user) {
Future<?> future =
mailboxExecutor.submit(
() -> {
Thread.currentThread().setName("MailboxService-User-" + user.getId());
LOG.debug("Started new thread: {}", Thread.currentThread().getName());
mailboxConnectionHandler.listenToMailbox(user);
});
mailboxFutures.put(user.getId(), future);
}
MailboxConnectionHandler
void listenToMailbox(User user) throws MessagingException {
Settings settings = user.getSettings();
Properties properties = getProperties(settings);
Store store = createStoreSession(properties, user);
IMAPFolder inbox = openInbox(store);
addMessageCountListener(inbox, user);
LOG.debug("Issuing IMAP idle command for mailbox of user {}.", user.getId());
inbox.idle();
}
After reading some posts on stack overflow and the IMAP Idle RFC I need to be able to close the inbox of a user in order to break the idle command therefore I want to store the IMAPFolder inbox of each user to be able to do that.
I thought of using private static final ThreadLocal<IMAPFolder> inboxThreadLocal = new ThreadLocal<>(); to set it but since I am using an executor service I think is is unsafe because it's a thread pool and threads are being reused therefore I am concerned about a thread using a "wrong / old" inbox.
What is the correct way to store thread specific variables when using an executor service as I described it?
One approach I thought of was using ConcurrentHashMap<Long, IMAPFolder> with the user Id being the key.
I thought of using
private static final ThreadLocal<IMAPFolder> inboxThreadLocal = new ThreadLocal<>();to set it but since I am using an executor service I think is is unsafe because it's a thread pool and threads are being reused therefore I am concerned about a thread using a "wrong / old" inbox.
That's not necessarily unsafe. If you opened all the folders you were ever going to open before you closed any, then there would be no de facto reuse of those threadlocals. But that would be clunky, and even if it would be safe now, with your present design, it would be a risk for breaking later. The IMAPFolder data you want to track are task-specific, so if your threads are not (or might not be) also task specific then the threads are the wrong place to keep it.
At this point, then, you need to recognize that using a lambda to specify the task for a thread is a convenience feature, not a requirement. You should not feel obliged to do that if it gets in the way. The generalization is to write a class that implements Runnable or Callable (choose one) to represent the task, and to provide instances of that class to your executor service. If you make the IMAPFolder for each task an instance variable of the class then you will control its scope and lifetime and make it easily available to the code that needs to access it.
You can also give the task class whatever constructor(s) and other methods are appropriate, and by keeping track of them separately from the executor service, you could exercise query and control, such as inquiring about numbers of unread messages or instructing them to shut down.