harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexey Petrenko" <alexey.a.petre...@gmail.com>
Subject Re: [classlib][nio] jetty corrupts big static files
Date Sat, 08 Dec 2007 21:39:50 GMT
+1 for commit.

2007/12/9, Tim Ellison <t.p.ellison@gmail.com>:
> 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 <mark.hindess@googlemail.com> 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) {
> >
>

Mime
View raw message