davsclaus commented on code in PR #25907:
URL: https://github.com/apache/camel/pull/25907#discussion_r3892145683
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -162,35 +162,40 @@ protected int poll() throws Exception {
// okay consumer is connected to the mail server
forceConsumerAsReady();
+ // capture folder into a local variable so any later field change
(e.g. on stop)
+ // does not affect the in-progress poll
+ final Folder currentFolder = folder;
+
try {
- int count = folder.getMessageCount();
+ int count = currentFolder.getMessageCount();
if (count > 0) {
Queue<Exchange> messages = retrieveMessages();
polledMessages = processBatch(CastUtils.cast(messages));
final MailBoxPostProcessAction postProcessor =
getEndpoint().getPostProcessAction();
if (postProcessor != null) {
- postProcessor.process(folder);
+ postProcessor.process(currentFolder);
}
} else if (count == -1) {
- throw new MessagingException("Folder: " + folder.getFullName()
+ " is closed");
+ throw new MessagingException("Folder: " +
currentFolder.getFullName() + " is closed");
}
} catch (Exception e) {
handleException(e);
} finally {
// need to ensure we release resources, but only if closeFolder or
disconnect = true
if (getEndpoint().getConfiguration().isCloseFolder() ||
getEndpoint().getConfiguration().isDisconnect()) {
try {
- if (folder != null && folder.isOpen()) {
+ if (currentFolder != null && currentFolder.isOpen()) {
if (LOG.isDebugEnabled()) {
- LOG.debug("Close mailbox folder {} from {}",
folder.getName(),
+ LOG.debug("Close mailbox folder {} from {}",
currentFolder.getName(),
getEndpoint().getConfiguration().getMailStoreLogInformation());
}
- folder.close(true);
+ currentFolder.close(true);
}
} catch (Exception e) {
// some mail servers will lock the folder so we ignore in
this case (CAMEL-1263)
- LOG.debug("Could not close mailbox folder: {}. This
exception is ignored.", folder.getName(), e);
+ LOG.debug("Could not close mailbox folder: {}. This
exception is ignored.",
+ currentFolder != null ? currentFolder.getName() :
"null", e);
Review Comment:
Minor: this null check (and the one at `if (currentFolder != null &&
currentFolder.isOpen())` a few lines up) can never take the "null" branch
within a single `poll()` invocation. `currentFolder` is a `final` local
assigned once from `folder`, and `poll()` already throws
`IllegalStateException` earlier (`if (store == null || folder == null)`) if
`folder` is null at that point — so `currentFolder` is guaranteed non-null for
the rest of the method. The actual fix here is the local-variable capture
itself (it structurally prevents a concurrent `disconnect()` from affecting
this in-flight `poll()`), not the null guard. Not a bug, but the ternary is
effectively dead code and could be simplified to `currentFolder.getName()` — or
keep it if you'd rather stay defensive against future refactors that might
reorder the capture.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
