* Removed unused variable;
* Removed a couple of naked notifies;
* Fixed a type cast;
* Removed an unnecessary exception catch;
* Replaced numerical constructor with valueOf in a couple of places.
The assignee is responsible for closing (committing) this patch (see ZooKeeperPatches on wiki) -- this includes getting additional review if necessary.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
1) On line 214/339 synchronization was removed, don't you have to hold the monitor when calling notify? "This method should only be called by a thread that is the owner of this object's monitor."
2) Remove unused field line 377
3) This is a new one to me -- Findbugs has an interesting comment for lines 536, 567
"[DL] Synchronization on boxed primative could lead to deadlock [DL_SYNCHRONIZATION_ON_BOXED_PRIMITIVE]
The code synchronizes on a boxed primitive constant, such as an Integer.
Since Integer objects can be cached and shared, this code could be synchronizing on the same object as other, unrelated code, leading to unresponsiveness and possible deadlock"
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Have taken the following actions:
* Put back the synchronized blocks;
* Have double-checked that the naked notifies are actually ok. Findbugs complains about no change to the state of the object, which is true. The notify is triggered by the reception of a message, though, which is an external event, and that's why there is no state change upon the execution of the notify.
File Added: patch-auth-le
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Patch version 1
Logged In: YES
user_id=12853
Originator: NO
The assignee is responsible for closing (committing) this patch (see ZooKeeperPatches on wiki) -- this includes getting additional review if necessary.
Logged In: YES
user_id=12853
Originator: NO
1) On line 214/339 synchronization was removed, don't you have to hold the monitor when calling notify? "This method should only be called by a thread that is the owner of this object's monitor."
2) Remove unused field line 377
3) This is a new one to me -- Findbugs has an interesting comment for lines 536, 567
"[DL] Synchronization on boxed primative could lead to deadlock [DL_SYNCHRONIZATION_ON_BOXED_PRIMITIVE]
The code synchronizes on a boxed primitive constant, such as an Integer.
private static Integer count = 0;
...
synchronized(count) {
count++;
}
...
Since Integer objects can be cached and shared, this code could be synchronizing on the same object as other, unrelated code, leading to unresponsiveness and possible deadlock"
patch version 2
Logged In: YES
user_id=1926444
Originator: YES
Have taken the following actions:
* Put back the synchronized blocks;
* Have double-checked that the naked notifies are actually ok. Findbugs complains about no change to the state of the object, which is true. The notify is triggered by the reception of a message, though, which is an external event, and that's why there is no state change upon the execution of the notify.
File Added: patch-auth-le
Logged In: YES
user_id=1926680
Originator: NO
looks good.. pat you ok with it?
Logged In: YES
user_id=12853
Originator: NO
+1 - fine w/me (this comment is on patch v2 from the downloads, not patchv1)
Logged In: YES
user_id=1926680
Originator: NO
Committed revision 155.
Logged In: YES
user_id=1926680
Originator: NO
Committed revision 155.