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 Fri, 07 Dec 2007 13:56:56 GMT
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