[Zookeeper-developer] [ zookeeper-Patches-1956801 ] Small fixes to Authenticated LE

[SourceForge.net <no...@so...>]

Patches item #1956801, was opened at 2008-05-03 17:00
Message generated for change (Comment added) made by mahadevkonar
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1956801&group_id=209147

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
>Status: Closed
>Resolution: Fixed
Priority: 7
Private: No
Submitted By: fpj (fpj)
Assigned to: Mahadev Konar (mahadevkonar)
Summary: Small fixes to Authenticated LE

Initial Comment:
* 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.

----------------------------------------------------------------------

>Comment By: Mahadev Konar (mahadevkonar)
Date: 2008-05-08 20:56

Message:
Logged In: YES 
user_id=1926680
Originator: NO

Committed revision 155.


----------------------------------------------------------------------

Comment By: Patrick Hunt (phunt)
Date: 2008-05-08 20:25

Message:
Logged In: YES 
user_id=12853
Originator: NO

+1 - fine w/me (this comment is on patch v2 from the downloads, not
patchv1)


----------------------------------------------------------------------

Comment By: Mahadev Konar (mahadevkonar)
Date: 2008-05-08 20:06

Message:
Logged In: YES 
user_id=1926680
Originator: NO

looks good.. pat you ok with it?


----------------------------------------------------------------------

Comment By: fpj (fpj)
Date: 2008-05-07 01:16

Message:
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

----------------------------------------------------------------------

Comment By: Patrick Hunt (phunt)
Date: 2008-05-06 18:50

Message:
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"


----------------------------------------------------------------------

Comment By: Patrick Hunt (phunt)
Date: 2008-05-06 17:41

Message:
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.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1956801&group_id=209147