You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2015/11/05 18:30:13 UTC
[Bug 58590] New: org.apache.catalina.realm.MemoryRealm can use
backgroundProcess() to reload tomcat-users.xml when it changes
https://bz.apache.org/bugzilla/show_bug.cgi?id=58590
Bug ID: 58590
Summary: org.apache.catalina.realm.MemoryRealm can use
backgroundProcess() to reload tomcat-users.xml when it
changes
Product: Tomcat 9
Version: unspecified
Hardware: All
OS: All
Status: NEW
Severity: enhancement
Priority: P2
Component: Catalina
Assignee: dev@tomcat.apache.org
Reporter: apache@leocap.com
Based on a derived class that I created and tested successfully with Tomcat 8,
I suggest the following lines be added to MemoryRealm in order to give it this
capability:
private Date _lastUpdate = null;
private File _usersFile = null;
public MemoryRealm()
{
_lastUpdate = new Date();
_usersFile = new File( getPathname() );
}
/**
* The default 10 second value for the Engine's backgroundProcessorDelay
XML attribute
* will cause this method to be called that often.
*/
@Override
public void backgroundProcess()
{
try {
// only reload if file has changed since we last loaded it
if( _lastUpdate.getTime() < _usersFile.lastModified() ) {
_lastUpdate = new Date();
log.info( "reloading " + getPathname() );
stop();
principals.clear();
start(); // trigger a repopulation (from tomcat-users.xml)
}
}
catch( Exception ex ) {
log.error( "Failed to re-initialise realm: ", ex );
}
}
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58590] org.apache.catalina.realm.MemoryRealm can use
backgroundProcess() to reload tomcat-users.xml when it changes
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58590
--- Comment #2 from Aidan <ap...@leocap.com> ---
(In reply to Konstantin Kolinko from comment #1)
Hi Konstantin,
> 1. Code conventions:
> http://tomcat.apache.org/getinvolved.html
>
> Actually Sun's conventions but with spaces instead of tabs.
Ok, I can edit to conform with that.
> 2. backgroundProcess() runs frequently. I see no need to perform this work
> on each run.
Every 10 seconds for this container, by default, I believe - that doesn't seem
too often to merely check the modified date of a file.
> 3. It shall work without stopping and starting the realm. Users should not
> be locked from the system while it reloads.
OK, fair point, I can try to address that.
> 4. MemoryRealm is rarely used. The usual configuration uses
> UserDatabaseRealm + MemoryUserDatabase (created by
> MemoryUserDatabaseFactory).
True, but it wasn't clear to me how to effect the same change in
UserDatabaseRealm and I was a bit pushed for time.
> 5. It should be possible to turn this feature on or off.
As I wasn't proposing this change for the default Realm I didn't think that was
necessary.
> I do not know what the default should be.
> On one hand we already have <Host autoDeploy="true"> so we are already
> checking hard drive by default.
>
> On other hand, on production systems such setting is likely to be off by
> default (as there is an expectation that nobody will ever update that file).
I think that setting only relates to .war files ?? AFAIK there is currently no
mechanism to force re-reading of tomcat-users.xml without a restart. But I
accept that if a user requires runtime user/role loading then they should
probably use a JNDI or JDBC-based realm instead. However, I think it's nice to
have an out-of-the-box alternative that has this capability.
I wrote this patch at the bank where I work where we use Tomcat (with the APR)
to serve several hundred internal users 24x5. We rarely update tomcat-users.xml
but when we do we have to perform a disruptive restart. The patch fixes that,
at least.
> 6. There shall be an explicit method to reload and an explicit method to
> perform an up-to-date check - so that it were possible to call them via JMX.
I hadn't thought of that. But as cool and useful as JMX is, it's beyond a lot
of the casual users whom this patch is aimed at, I suspect.
Having said all that, I'm happy to rework it as best I can in order to address
all your points or for you to simply reject it.
Kind Regards,
Aidan
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58590] org.apache.catalina.realm.MemoryRealm can use
backgroundProcess() to reload tomcat-users.xml when it changes
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58590
--- Comment #3 from Christopher Schultz <ch...@christopherschultz.net> ---
Two things:
1. If possible, look at the UserDatabase class to see if you can get that to
reload instead of the Realm. If you think about e.g. DatsSourceRealm, it makes
more sense to "reload" the data source and not the realm itself.
2. Instead of calling _lastUpdate.getTime() each time, why not just store the
native long value returned by it? (Also, you can then simply store the value of
_usersFile.lastModified instead of creating a new Date object).
As for Konstantin being picky about your implementation... we get to be as
picky about patch submissions as the submitters are willing to tolerate. Since
you are motivated to get your patch accepted, we just want to make sure it's as
high-quality as possible.
Thanks for your contributions!
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58590] org.apache.catalina.realm.MemoryRealm can use
backgroundProcess() to reload tomcat-users.xml when it changes
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58590
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #4 from Mark Thomas <ma...@apache.org> ---
Implemented in trunk for 9.0.13 onwards.
Not back-ported as it would require changing the UserDatabase API as Java 7
doesn't support default method implementations.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58590] org.apache.catalina.realm.MemoryRealm can use
backgroundProcess() to reload tomcat-users.xml when it changes
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58590
recrute <re...@163.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
Resolution|FIXED |---
--- Comment #5 from recrute <re...@163.com> ---
I just switch tomcat version from apache-tomcat-9.0.11 to apache-tomcat-9.0.13,
and then I found the tomcat instance could not work well again.
The error log shown:
org.apache.tomcat.jni.Error: 24: Too many open files
And here is the data from lsof:
[root@localhost versions]# lsof|grep "java"|wc -l
194186
[root@localhost versions]# lsof|grep "java"|grep "conf/tomcat-users.xml"|wc -l
152991
We can see there are too many handles from tomcat about "tomcat-users.xml",
so is there any wrong about apache-tomcat-9.0.13 on this case?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58590] org.apache.catalina.realm.MemoryRealm can use
backgroundProcess() to reload tomcat-users.xml when it changes
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58590
--- Comment #1 from Konstantin Kolinko <kn...@gmail.com> ---
1. Code conventions:
http://tomcat.apache.org/getinvolved.html
Actually Sun's conventions but with spaces instead of tabs.
2. backgroundProcess() runs frequently. I see no need to perform this work on
each run.
3. It shall work without stopping and starting the realm. Users should not be
locked from the system while it reloads.
4. MemoryRealm is rarely used. The usual configuration uses UserDatabaseRealm
+ MemoryUserDatabase (created by MemoryUserDatabaseFactory).
5. It should be possible to turn this feature on or off.
I do not know what the default should be.
On one hand we already have <Host autoDeploy="true"> so we are already checking
hard drive by default.
On other hand, on production systems such setting is likely to be off by
default (as there is an expectation that nobody will ever update that file).
6. There shall be an explicit method to reload and an explicit method to
perform an up-to-date check - so that it were possible to call them via JMX.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58590] org.apache.catalina.realm.MemoryRealm can use
backgroundProcess() to reload tomcat-users.xml when it changes
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58590
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|REOPENED |RESOLVED
--- Comment #6 from Mark Thomas <ma...@apache.org> ---
See bug 62924.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org