harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Ellison <t.p.elli...@gmail.com>
Subject Re: [classlib][nio] jetty corrupts big static files
Date Sat, 08 Dec 2007 22:51:00 GMT
Alexey Petrenko wrote:
> +1 for commit.

Thanks Alexey -- committed at r602572.

Regards,
Tim



> 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