
| Key: |
UP-1487
|
| Type: |
Bug
|
| Status: |
Open
|
| Priority: |
Major
|
| Assignee: |
Unassigned
|
| Reporter: |
Warren Liang
|
| Votes: |
0
|
| Watchers: |
1
|
|
If you were logged in you would be able to see more operations.
|
|
|
|
Original Estimate:
|
Unknown
|
Remaining Estimate:
|
Unknown
|
Time Spent:
|
Unknown
|
|
|
In RDBMDistributedLayoutStore._safeGetUserLayout, an IllegalStateException may be thrown from the statement for the line "releaseReadLock(person);" This is caused during situations of high load when multiple threads are loading the layout document of the same user.
A read lock (EDU.oswego.cs.dl.util.concurrent.ReentrantWriterPreferenceReadWriteLock) is used by the _safeGetUserLayout method. However, the getReadWriteLock method used to create this lock is not thread safe. A synchronized hash table is used to store a lock for each user ID.
Each lock contains a mapping between a thread and the number of read locks from that thread. If a thread tries to release a read lock and no locks to that instance of the lock was made, then an ISE is thrown.
getReadWriteLock does the following (summarized):
1) getLock for the user ID (synchronized operation)
2) if lock is null,
a) create a new lock
b) place lock into hash map (synchronized operation)
3) return lock
The problem with this that given unfavorable timing conditions, two lock instances may be created because the getting of a lock and its creation are not an atomic operation.
Example:
thread A executes steps 1, 2a
thread B executes steps 1, 2a, 2b, 3
thread A executes steps 2b, 3
In this case, you can see that two threads create two locks, and this will cause a problem in the future when locks are released, because thread B's read lock was acquired on the lock instance created in thread B, but B's instance is overwritten by thread A when it comes time to release the lock.
Possible and not fully tested solution: synchronize the getReadWriteLock method. I tried synchronizing to the (this) object, but experienced some deadlock. Then I tried synchronizing to the hash table, and that seemed to work better, but it hasn't been fully tested yet.
There is also a minor memory leak. There is no way to remove locks from memory, so locks will persist in memory. The number of locks is equal to the number of unique user IDs that have logged on.
It's been a long time since I've taken operating systems, but if only read locks and no write locks are used, it seems like the whole locking scheme is unnecessary.
|
|
Description
|
In RDBMDistributedLayoutStore._safeGetUserLayout, an IllegalStateException may be thrown from the statement for the line "releaseReadLock(person);" This is caused during situations of high load when multiple threads are loading the layout document of the same user.
A read lock (EDU.oswego.cs.dl.util.concurrent.ReentrantWriterPreferenceReadWriteLock) is used by the _safeGetUserLayout method. However, the getReadWriteLock method used to create this lock is not thread safe. A synchronized hash table is used to store a lock for each user ID.
Each lock contains a mapping between a thread and the number of read locks from that thread. If a thread tries to release a read lock and no locks to that instance of the lock was made, then an ISE is thrown.
getReadWriteLock does the following (summarized):
1) getLock for the user ID (synchronized operation)
2) if lock is null,
a) create a new lock
b) place lock into hash map (synchronized operation)
3) return lock
The problem with this that given unfavorable timing conditions, two lock instances may be created because the getting of a lock and its creation are not an atomic operation.
Example:
thread A executes steps 1, 2a
thread B executes steps 1, 2a, 2b, 3
thread A executes steps 2b, 3
In this case, you can see that two threads create two locks, and this will cause a problem in the future when locks are released, because thread B's read lock was acquired on the lock instance created in thread B, but B's instance is overwritten by thread A when it comes time to release the lock.
Possible and not fully tested solution: synchronize the getReadWriteLock method. I tried synchronizing to the (this) object, but experienced some deadlock. Then I tried synchronizing to the hash table, and that seemed to work better, but it hasn't been fully tested yet.
There is also a minor memory leak. There is no way to remove locks from memory, so locks will persist in memory. The number of locks is equal to the number of unique user IDs that have logged on.
It's been a long time since I've taken operating systems, but if only read locks and no write locks are used, it seems like the whole locking scheme is unnecessary. |
Show » |
| There are no comments yet on this issue.
|
|