[
https://issues.apache.org/activemq/browse/AMQ-1254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_39495
]
solprovider edited comment on AMQ-1254 at 6/23/07 3:01 PM:
-----------------------------------------------------------
The reason amq is using this poor design is a programmer did not read the docs
for the System and Properties classes: "Because Properties inherits from
Hashtable, the put and putAll methods can be applied to a Properties object.
Their use is strongly discouraged as they allow the caller to insert entries
whose keys or values are not Strings. The setProperty method should be used
instead." ActiveMQ even contains code that will break if System.Properties
contains elements that cannot be cast to String.
To fix this:
1. Discard function getVmLockSet(). The locks cannot use a Set.
2. Change lock() and unlock() to this code:
{code:title=Good Locking|borderStyle=solid}
private synchronized void lock() throws IOException {
if (!disableLocking && directory != null && lock == null) {
String key = getClass().getName() + ".lock." +
directory.getCanonicalPath().replaceAll("[:/\\\\]", ".");
String property = System.getProperty(key);
if ((null == property) || ("" == property)) {
if (!brokenFileLock) {
lock = rootIndexManager.getLock();
if (lock == null) {
throw new StoreLockedExcpetion("Kaha Store " + directory.getName() + " is
already opened by another application");
} else
System.setProperty(key, "LOCK");
//Could store datetime of lock.
}
} else { //already locked
throw new StoreLockedExcpetion("Kaha Store " + directory.getName() + " is
already opened by this application.");
}
}
}
private synchronized void unlock() throws IOException {
if (!disableLocking && (null != directory) && (null != lock)) {
String key = getClass().getName() + ".lock." +
directory.getCanonicalPath().replaceAll("[:/\\\\]", ".");
if (null != System.getProperty(key))
System.setProperty(key, "");
if (lock.isValid()) {
lock.release();
}
lock = null;
}
}
{code}
This should be faster since we are not retrieving a Set<String> (but it will
slow code that checks every System.Property.) This should use less memory since
we do not have the overhead of one Set<String>. This does not violate
System.Properties by bypassing its functions to use the parent HashMap
functions.
Warning: I am not set up to work with Java 1.5 (required by this class) so this
needs to be tested.
was:
The reason amq is using this poor design is a programmer did not read the docs
for the System and Properties classes: "Because Properties inherits from
Hashtable, the put and putAll methods can be applied to a Properties object.
Their use is strongly discouraged as they allow the caller to insert entries
whose keys or values are not Strings. The setProperty method should be used
instead." ActiveMQ even contains code that will break if System.Properties
contains elements that cannot be cast to String.
To fix this:
1. Discard function getVmLockSet(). The locks cannot use a Set.
2. Change lock() and unlock() to this code:
{code:title=Good Locking|borderStyle=solid}
private synchronized void lock() throws IOException {
if (!disableLocking && directory != null && lock == null) {
String key = getClass().getName() + ".lock." +
directory.getCanonicalPath().replaceAll(":/\\\\", ".");
String property = System.getProperty(key);
if ((null == property) || ("" == property)) {
if (!brokenFileLock) {
lock = rootIndexManager.getLock();
if (lock == null) {
throw new StoreLockedExcpetion("Kaha Store " + directory.getName() + " is
already opened by another application");
} else
System.setProperty(key, "LOCK");
//Could store datetime of lock.
}
} else { //already locked
throw new StoreLockedExcpetion("Kaha Store " + directory.getName() + " is
already opened by this application.");
}
}
}
private synchronized void unlock() throws IOException {
if (!disableLocking && (null != directory) && (null != lock)) {
String key = getClass().getName() + ".lock." +
directory.getCanonicalPath().replaceAll("[:/\\\\]", ".");
if (null != System.getProperty(key))
System.setProperty(key, "");
if (lock.isValid()) {
lock.release();
}
lock = null;
}
}
{code}
This should be faster since we are not retrieving a Set<String> (but it will
slow code that checks every System.Property.) This should use less memory since
we do not have the overhead of one Set<String>. This does not violate
System.Properties by bypassing its functions to use the parent HashMap
functions.
Warning: I am not set up to work with Java 1.5 (required by this class) so this
needs to be tested.
> Kaha Store puts a non-string into System properties
> ---------------------------------------------------
>
> Key: AMQ-1254
> URL: https://issues.apache.org/activemq/browse/AMQ-1254
> Project: ActiveMQ
> Issue Type: Bug
> Affects Versions: 4.1.2
> Reporter: David Jencks
> Attachments: Kaha-Store.patch
>
>
> KahaStore puts a hashmap into SystemProperties, which causes problems with
> programs that expect only strings as properties. In particular some versions
> of Hibernate assume all system properties are strings: this is causing
> difficulties running roller in geronimo 2.0
> Attached is a proposed solution. I have no idea how to test it. I get the
> same 7 failures and one error building amq with and without the change.
> The proposal stores the list of locked directories in a string and converts
> it back and forth to a map whenever it is accessed. I use a constant string
> as the vm-wide lock monitor formerly provided by the HashSet. According to
> the String javadoc constant strings are intern()ed and the same instance is
> provided in any classloader: this makes it suitable for a vm-wide lock
> monitor.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
