You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by "David Jencks (JIRA)" <ji...@apache.org> on 2007/05/29 00:41:32 UTC
[jira] Created: (AMQ-1254) Kaha Store puts a non-string into System
properties
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.
[jira] Reopened: (AMQ-1254) Kaha Store puts a non-string into
System properties
Posted by "David Jencks (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Jencks reopened AMQ-1254:
-------------------------------
Committed improved fix in 4.1 branch rev 640659.
The trunk fix completely ignores the main point of the lock, which is to prevent copies of activemq in the same vm but in different classloaders from attempting to use the same file. The branch fix locks on a constant string, which according to the java docs is interned and thus guaranteed to be the same object across classloaders.
> 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
> Assignee: Rob Davies
> Fix For: 5.0.0
>
> 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.
[jira] Issue Comment Edited: (AMQ-1254) Kaha Store puts a
non-string into System properties
Posted by "solprovider (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Resolved: (AMQ-1254) Kaha Store puts a non-string into
System properties
Posted by "Rob Davies (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rob Davies resolved AMQ-1254.
-----------------------------
Resolution: Fixed
Fix Version/s: 5.0.0
SVN revision 558774
> 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
> Assignee: Rob Davies
> Fix For: 5.0.0
>
> 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.
[jira] Commented: (AMQ-1254) Kaha Store puts a non-string into
System properties
Posted by "Qian Su (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_39745 ]
Qian Su commented on AMQ-1254:
------------------------------
Does release 5 come with the fixed version of activeio, i.e. property org.apache.activeio.journal.active.lockMap is set to string properly? As far as I know, release 4.1.1 comes with activeio-3.0.0-incubator which has the bug and their current trunk has it fixed.
Thx!
> 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
> Assignee: Rob Davies
> Fix For: 5.0.0
>
> 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.
[jira] Commented: (AMQ-1254) Kaha Store puts a non-string into
System properties
Posted by "David Jencks (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_39235 ]
David Jencks commented on AMQ-1254:
-----------------------------------
Apparently there's a similar problem with a property called org.apache.activeio.journal.active.lockMap
> 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.
[jira] Commented: (AMQ-1254) Kaha Store puts a non-string into
System properties
Posted by "solprovider (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_39495 ]
solprovider commented on AMQ-1254:
----------------------------------
The reason amq is using this poor design is a programmer did not read the docs for the System class. ActiveMQ 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>. 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.
[jira] Issue Comment Edited: (AMQ-1254) Kaha Store puts a
non-string into System properties
Posted by "solprovider (JIRA)" <ji...@apache.org>.
[ 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 2:59 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.
[jira] Issue Comment Edited: (AMQ-1254) Kaha Store puts a
non-string into System properties
Posted by "solprovider (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Issue Comment Edited: (AMQ-1254) Kaha Store puts a
non-string into System properties
Posted by "solprovider (JIRA)" <ji...@apache.org>.
[ 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:05 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.
The replaceAll() may not be necessary. That line may be moved to a getPropertyKey() function since it is used in both of these 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.
[jira] Issue Comment Edited: (AMQ-1254) Kaha Store puts a
non-string into System properties
Posted by "solprovider (JIRA)" <ji...@apache.org>.
[ 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 2:58 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 class. ActiveMQ 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>. 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.
[jira] Commented: (AMQ-1254) Kaha Store puts a non-string into
System properties
Posted by "David Jencks (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_39453 ]
David Jencks commented on AMQ-1254:
-----------------------------------
See also https://issues.apache.org/jira/browse/GERONIMO-3243
The reason amq is using this peculiar design is that they need a lock across the entire vm, not just within one classloader. Putting something in System.getProperties is a simple way to do that and it's not too easy to think of another way. However it is possible to change what the locked object is to a string, although at a slight cost.
> 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.
[jira] Issue Comment Edited: (AMQ-1254) Kaha Store puts a
non-string into System properties
Posted by "solprovider (JIRA)" <ji...@apache.org>.
[ 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:16 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 = getPropertyKey();
String property = System.getProperty(key);
if (null == 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)) {
System.getProperties().remove(getPropertyKey());
if (lock.isValid()) {
lock.release();
}
lock = null;
}
}
private String getPropertyKey(){
//Is replaceAll() needed? Should test without it.
return getClass().getName() + ".lock." + directory.getCanonicalPath().replaceAll("[:/\\\\]", ".");
}
{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.
The replaceAll() may not be necessary. That line may be moved to a getPropertyKey() function since it is used in both of these 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.
[jira] Commented: (AMQ-1254) Kaha Store puts a non-string into
System properties
Posted by "Qian Su (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_39690 ]
Qian Su commented on AMQ-1254:
------------------------------
Is there an estimated time to fix for setting property org.apache.activeio.journal.active.lockMap to String? Thx!
> 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.
[jira] Issue Comment Edited: (AMQ-1254) Kaha Store puts a
non-string into System properties
Posted by "solprovider (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_39379 ]
solprovider edited comment on AMQ-1254 at 6/13/07 11:25 AM:
------------------------------------------------------------
These issues are both related to locking systems. The HashSet and HashMap contain a list of locked items. This sounds troublesome because the classes are not synchronized. System.Properties is a HashTable (automatically synchronized.) System.Properties is being violated: "Each key and its corresponding value in the property list is a string." from http://java.sun.com/j2se/1.4.2/docs/api/java/util/Properties.html
{code:title=Test Code|borderStyle=solid}
boolean test(){ //true = passes, false = failed.
boolean test = true;
java.util.Properties properties = System.getProperties();
java.util.Enumeration enumeration = properties.elements();
while(test & enumeration.hasMoreElements()) test= String.class.equals(enumeration.nextElement().getClass());
enumeration = properties.keys();
while(test & enumeration.hasMoreElements()) test= String.class.equals(enumeration.nextElement().getClass());
return test;
}
{code}
The typical algorithm for locking is to put an identifier as a key into the central location. No objects are stored; use a static class if necessary, but this typically indicates poor design. The property value is not needed for the locking system and typically contains information about the lock such as who created it and when. So locking C:Windows\Temp\badcode.java could create:
KEY: org.apache.activeio.journal.active.lockMap.C%3aWindows%2cTemp%2cbadcode%2ejava
VALUE: 20070613T122219Z solprovider reason
Set locks with System.setProperty(key, informationString);
Get the information about a lock with System.getProperty(key);
Check locks with System.getProperties().containsKey(key);
Remove locks with System.getProperties().remove(key);
was:
These issues are both related to locking systems. The HashSet and HashMap contain a list of locked items. This sounds troublesome because the classes are not synchronized. System.Properties is a HashTable (automatically synchronized.) System.Properties is being violated: "Each key and its corresponding value in the property list is a string." from http://java.sun.com/j2se/1.4.2/docs/api/java/util/Properties.html
The typical algorithm for locking is to put an identifier as a key into the central location. No objects are stored; use a static class if necessary, but this typically indicates poor design. The property value is not needed for the locking system and typically contains information about the lock such as who created it and when. So locking C:Windows\Temp\badcode.java could create:
KEY: org.apache.activeio.journal.active.lockMap.C%3aWindows%2cTemp%2cbadcode%2ejava
VALUE: 20070613T122219Z solprovider reason
Set locks with System.setProperty(key, informationString);
Get the information about a lock with System.getProperty(key);
Check locks with System.getProperties().containsKey(key);
Remove locks with System.getProperties().remove(key);
> 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.
[jira] Commented: (AMQ-1254) Kaha Store puts a non-string into
System properties
Posted by "solprovider (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_39379 ]
solprovider commented on AMQ-1254:
----------------------------------
These issues are both related to locking systems. The HashSet and HashMap contain a list of locked items. This sounds troublesome because the classes are not synchronized. System.Properties is a HashTable (automatically synchronized.) System.Properties is being violated: "Each key and its corresponding value in the property list is a string." from http://java.sun.com/j2se/1.4.2/docs/api/java/util/Properties.html
The typical algorithm for locking is to put an identifier as a key into the central location. No objects are stored; use a static class if necessary, but this typically indicates poor design. The property value is not needed for the locking system and typically contains information about the lock such as who created it and when. So locking C:Windows\Temp\badcode.java could create:
KEY: org.apache.activeio.journal.active.lockMap.C%3aWindows%2cTemp%2cbadcode%2ejava
VALUE: 20070613T122219Z solprovider reason
Set locks with System.setProperty(key, informationString);
Get the information about a lock with System.getProperty(key);
Check locks with System.getProperties().containsKey(key);
Remove locks with System.getProperties().remove(key);
> 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.
[jira] Issue Comment Edited: (AMQ-1254) Kaha Store puts a
non-string into System properties
Posted by "solprovider (JIRA)" <ji...@apache.org>.
[ 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:22 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:
(These functions are in KahaStore.java from the trunk today 20070623.)
{code:title=Good Locking|borderStyle=solid}
private synchronized void lock() throws IOException {
if (!disableLocking && directory != null && lock == null) {
String key = getPropertyKey();
String property = System.getProperty(key);
if (null == 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)) {
System.getProperties().remove(getPropertyKey());
if (lock.isValid()) {
lock.release();
}
lock = null;
}
}
private String getPropertyKey(){
//Is replaceAll() needed? Should test without it.
return getClass().getName() + ".lock." + directory.getCanonicalPath().replaceAll("[:/\\\\]", ".");
}
{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 = getPropertyKey();
String property = System.getProperty(key);
if (null == 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)) {
System.getProperties().remove(getPropertyKey());
if (lock.isValid()) {
lock.release();
}
lock = null;
}
}
private String getPropertyKey(){
//Is replaceAll() needed? Should test without it.
return getClass().getName() + ".lock." + directory.getCanonicalPath().replaceAll("[:/\\\\]", ".");
}
{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.
[jira] Assigned: (AMQ-1254) Kaha Store puts a non-string into
System properties
Posted by "Rob Davies (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rob Davies reassigned AMQ-1254:
-------------------------------
Assignee: Rob Davies
> 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
> Assignee: Rob Davies
> 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.
[jira] Resolved: (AMQ-1254) Kaha Store puts a non-string into
System properties
Posted by "Rob Davies (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rob Davies resolved AMQ-1254.
-----------------------------
Fix Version/s: 5.1.0
(was: 5.0.0)
Resolution: Fixed
Fixed by SVN revision 646219
> 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
> Assignee: Rob Davies
> Fix For: 5.1.0
>
> 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.