harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Hindess <mark.hind...@googlemail.com>
Subject Re: [classlib][nio] jetty corrupts big static files
Date Fri, 07 Dec 2007 09:12:53 GMT

On 6 December 2007 at 16:31, Tim Ellison <t.p.ellison@gmail.com> 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.



Mime
View raw message