jbonofre commented on code in PR #1636:
URL: https://github.com/apache/activemq/pull/1636#discussion_r2750674138
##########
activemq-jaas/src/main/java/org/apache/activemq/jaas/ReloadableProperties.java:
##########
@@ -49,19 +49,25 @@ public synchronized Properties getProps() {
public synchronized ReloadableProperties obtained() {
if (reloadTime < 0 || (key.isReload() &&
hasModificationAfter(reloadTime))) {
- props = new Properties();
+ // Load into a local variable first to preserve old data if reload
fails
+ // If we assigned 'props = new Properties()' first and load()
throws IOException,
+ // we'd expose empty Properties. By loading into newProps first,
we preserve the
+ // old data if the reload fails.
+ final Properties newProps = new Properties();
Review Comment:
I don't see the global `props` updated with `newProps`. Should we cleanup
`props` ?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
