+1 for commit.
2007/12/9, Tim Ellison <[EMAIL PROTECTED]>:
> Given that Aleksey and I came up with essentially the same patch to help
> alleviate this instability -- is there any support for putting it into
> the code now (ahead of M4 testing)? I believe it is a relatively
> localized and minor change even at this stage.
>
> Regards,
> Tim
>
>
> Tim Ellison wrote:
> > Aleksey Shipilev wrote:
> >> Hi, Mark!
> >>
> >> On Dec 7, 2007 12:12 PM, Mark Hindess <[EMAIL PROTECTED]> wrote:
> >>>> It looks like the code in NIO selector could do with a good review,
> >>>> and at minimum a few comments if not a partial redesign to tidy up the
> >>>> data structures and which locks are required for which, etc.
> >>> I agree. Though it is tempting to revert the HARMONY-4869 optimizations
> >>> (see JIRA comment for the two svn diff commands to do this) since that
> >>> does seem marginally more stable to me.
> >> I wonder if fixing the current legacy selector will be easier - that's
> >> because HARMONY-4869 already has some tidying in data structures. I
> >> suspect there are synchronization problems while modifying the key
> >> already registered on the selector. Can you try wrap the modKey() [1]
> >> with synchronization as follows:
> >>
> >> void modKey(SelectionKey sk) {
> >> + synchronized (this) {
> >> + synchronized (keysSet) {
> >> // TODO: update indexes rather than recreate the key
> >> delKey(sk);
> >> addKey(sk);
> >> + }
> >> + }
> >> }
> >>
> >> ...and see the difference?
> >
> >
> > I came to the same conclusion and have been running with the following
> > patch for a while... I'll admit that I followed other places in grabbing
> > the selectedkeys lock too, and it may not be necessary:
> >
> > Index: main/java/common/org/apache/harmony/nio/internal/SelectorImpl.java
> > ===================================================================
> > --- main/java/common/org/apache/harmony/nio/internal/SelectorImpl.java
> > (revision 602067)
> > +++ main/java/common/org/apache/harmony/nio/internal/SelectorImpl.java
> > (working copy)
> > @@ -359,8 +359,14 @@
> > */
> > void modKey(SelectionKey sk) {
> > // TODO: update indexes rather than recreate the key
> > - delKey(sk);
> > - addKey(sk);
> > + synchronized (this) {
> > + synchronized (keysSet) {
> > + synchronized (selectedKeys) {
> > + delKey(sk);
> > + addKey(sk);
> > + }
> > + }
> > + }
> > }
> >
> > /**
> > @@ -578,6 +584,9 @@
> > return unaddableSelectedKeys;
> > }
> >
> > + /*
> > + * Assumes calling thread holds locks on 'this', 'keysSet', and
> > 'selectedKeys'.
> > + */
> > private void doCancel() {
> > Set<SelectionKey> cancelledKeys = cancelledKeys();
> > synchronized (cancelledKeys) {
> >
>
