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 21:31:56 GMT
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