On 6 December 2007 at 16:31, Tim Ellison <[EMAIL PROTECTED]> wrote: > Aleksey Shipilev wrote: > > So far I've managed to reproduce this issue on SLES10 and > > epoll()-based Selector. There are really weird results difference > > between RHEL and SLES on the same build - I'm looking into the > > differences on the native side calls now. As far as you might be more > > familiar with legacy selector code, you might try to reproduce and > > catch the problem there, then we can sync our visions on what's going > > Aleksey, > > 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. > Given this is not a regression from M3, I suggest we downgrade it from > critical to major, and put off any significant reworking until after > the milestone. > > WDYT? Seems reasonable. I've started looking for obvious errors. I used the IBM VME with: java -Xcheck:jni:all,pedantic,nonfatal -jar start.jar to look for JNI issues and noticed several problems with the code for throwJavaNetSocketException in nethelp.c: 1) errorMessageString is created but not used in the common code path 2) in the HYPORT_ERROR_SOCKET_WOULDBLOCK code path, several JNI calls are not checked for exceptions - two NewObject, GetMethodID, CallObjectMethod 3) in the HYPORT_ERROR_SOCKET_WOULDBLOCK code path Throw is called but there is no return so the code simply falls through to the default case. It is tempting to fix this by adding a return but this code is effectively not being used so perhaps we should remove it? The code for 2 and 3 came from HARMONY-815 please can the originator take a look. A more minor issue in Java_org_apache_harmony_luni_platform_OSNetworkSystem_selectImpl is that after the poll we are setting outFlags to SOCKET_OP_NONE which is zero and since this array is initialised to zero anyway then this is a little pointless. Better to remove these. We can also then keep a count of any changes made and do use ReleaseIntArrayElements with JNI_ABORT if there are no changes - which seems to be a common case in the jetty test. Regards, Mark. P.S. check:jni is really useful we should test more of our native code with it.
