Alexey Petrenko wrote:
> +1 for commit.
Thanks Alexey -- committed at r602572.
Regards,
Tim
> 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) {
>>>
>
