You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Andrew Zhang <zh...@gmail.com> on 2006/08/29 03:41:11 UTC
Re: [jira] Created: (HARMONY-1295) [classlib][net] flaw in setReuseAddrAndReusePort()
On 8/28/06, Alexey Varlamov (JIRA) <ji...@apache.org> wrote:
>
> [classlib][net] flaw in setReuseAddrAndReusePort()
> --------------------------------------------------
>
> Key: HARMONY-1295
> URL: http://issues.apache.org/jira/browse/HARMONY-1295
> Project: Harmony
> Issue Type: Bug
> Components: Classlib
> Reporter: Alexey Varlamov
> Priority: Critical
>
>
> The tests.api.java.net.DatagramSocketTest test crashes on assert in DRLVM
> : Assertion `IsInstanceOf(env, obj, struct_Class_to_jclass(f->get_class()))'
> failed.
> The reason is that java.net.DatagramSocket.setReuseAddress(boolean) calls
> to
> PlainDatagramSocketImpl.setOption(int optID, Object val) with Boolean
> value, but underlying native code treats this value as Integer.
>
> Quickfix must be straightforward enough, but I inclined to blame the
> design of INetworkSystem.[set|get]SocketOption() as the actual root of the
> evil. The problem is that this API conceals the details of contract for
> particular options, and a client have to guess which type of value to pass.
I think it's "determine", not "guess". :)
And the implementation is basically an ugly switch sorting requests out to
> setBoolSocketOption() or setIntegerSocketOption(), etc. Shouldn'te refactor
> this?
Any proposals? Thanks!
Native stack is :
> vmcore.dll!GetIntField(JNIEnv_External * env=0x103353a8, _jobject *
> obj=0x0654f184, _jfieldID * fieldID=0x05e4c088) Line 304 + 0x3a C++
> hyluni.dll!intValue(const JNINativeInterface_ * * env=0x103353a8, _jobject
> * anInteger=0x0654f184) Line 598 + 0x1d C
> hyluni.dll!setReuseAddrAndReusePort(const JNINativeInterface_ * *
> env=0x103353a8, hysocket_struct * hysocketP=0x00160ae0, _jobject *
> optVal=0x0654f184) Line 1140 C
>
> hyluni.dll!Java_org_apache_harmony_luni_platform_OSNetworkSystem_setSocketOptionImpl(const
> JNINativeInterface_ * * env=, _jobject * thisClz=, _jobject *
> aFileDescriptor=, int anOption=, _jobject * aValue=) Line 2321 + 0xc C
>
> Java stack is:
> Stack Trace (003FB238):
> [003FB238] 00000000(n):
> org/apache/harmony/luni/platform/OSNetworkSystem.setSocketOptionImpl(Ljava
> /io/FileDescriptor;ILjava/lang/Object;)V
> [003FB238] 011D0625(m):
> org/apache/harmony/luni/platform/OSNetworkSystem.setSocketOption(Ljava/io/
> FileDescriptor;ILjava/lang/Object;)V
> [003FB238] 011CFF4E(m):
> org/apache/harmony/luni/net/PlainDatagramSocketImpl.setOption(ILjava/lang/
> Object;)V
> [003FB238] 011E3DB1(m): java/net/DatagramSocket.setReuseAddress(Z)V
> [003FB238] 011E3BB0(m):
> tests/api/java/net/DatagramSocketTest.test_getReuseAddress()V
> [003FB238] 100657D0(n):
> java/lang/reflect/VMReflection.invokeMethod(Ljava/lang/Object;Ljava/lang/O
> bject;[Ljava/lang/Object;)Ljava/lang/Object;
> [003FB238] 011C2CA0(m):
> java/lang/reflect/Method.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljav
> a/lang/Object;
> [003FB238] 011CCB25(m): junit/framework/TestCase.runTest()V
> [003FB238] 011CC66E(m): junit/framework/TestCase.runBare()V
> [003FB238] 011CC5BA(m): junit/framework/TestResult$1.protect()V
> [003FB238] 011CC44F(m):
> junit/framework/TestResult.runProtected(Ljunit/framework/Test;Ljunit/frame
> work/Protectable;)V
> [003FB238] 011CAA3C(m):
> junit/framework/TestResult.run(Ljunit/framework/TestCase;)V
> [003FB238] 011CA910(m):
> junit/framework/TestCase.run(Ljunit/framework/TestResult;)V
> [003FB238] 011CA85B(m):
> junit/framework/TestSuite.runTest(Ljunit/framework/Test;Ljunit/framework/T
> estResult;)V
> [003FB238] 011CA12E(m):
> junit/framework/TestSuite.run(Ljunit/framework/TestResult;)V
> [003FB238] 011C9B22(m):
> junit/textui/TestRunner.doRun(Ljunit/framework/Test;Z)Ljunit/framework/Tes
> tResult;
> [003FB238] 011C4186(m):
> junit/textui/TestRunner.start([Ljava/lang/String;)Ljunit/framework/TestRes
> ult;
> [003FB238] 011C365E(m): junit/textui/TestRunner.main([Ljava/lang/String;)V
> [003FB238] 100657D0(n):
> java/lang/reflect/VMReflection.invokeMethod(Ljava/lang/Object;Ljava/lang/O
> bject;[Ljava/lang/Object;)Ljava/lang/Object;
> [003FB238] 011C2CA0(m):
> java/lang/reflect/Method.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljav
> a/lang/Object;
> [003FB238] 011A59DF(m): java/lang/VMStart$MainThread.runImpl()V
> [003FB238] 100657D0(n): *null*
> End Stack Trace (003FB238, depth=22)
>
> --
> This message is automatically generated by JIRA.
> -
> If you think it was sent incorrectly contact one of the administrators:
> http://issues.apache.org/jira/secure/Administrators.jspa
> -
> For more information on JIRA, see: http://www.atlassian.com/software/jira
>
>
>
--
Andrew Zhang
China Software Development Lab, IBM
Re: [jira] Created: (HARMONY-1295) [classlib][net] flaw in setReuseAddrAndReusePort()
Posted by Alexey Varlamov <al...@gmail.com>.
[snip]
>
> Interesting, deep look into code, I find JNI use pointer as the field
> ID, so field-ID of value in Boolean is the same as the one of Integer.
> In this way the code can get a value field of Integer out of a Boolean
> (C pointer is great! :) ). IMO, this is a mis-use but cause no error
> until a strict check is taken. :)
Yes, this appears to be an (unwritten) standard de-facto. DRLVM also
uses fieldIDs to unwrap boxed values, and would silently bear with
such misuse in release mode.
> This is a bug of native code, I agree we should use "booleanValue"
> instead of "intValue", may we raise a JIRA and fix?
JIRA is already there, feel free to attach a patch :) We could apply a
quickfix and continue with design discussion without closing the JIRA.
--
Alexey
> > --
> >> Regards,
> >> Alexey
> >>
> >> ---------------------------------------------------------------------
> >> Terms of use : http://incubator.apache.org/harmony/mailing.html
> >> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> >> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >>
> >>
> >
> >
>
>
> --
>
> Best Regards!
>
> Jimmy, Jing Lv
> China Software Development Lab, IBM
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: [jira] Created: (HARMONY-1295) [classlib][net] flaw in setReuseAddrAndReusePort()
Posted by Paulex Yang <pa...@gmail.com>.
Jimmy, Jing Lv wrote:
> Andrew Zhang wrote:
>> On 8/29/06, Alexey Varlamov <al...@gmail.com> wrote:
>>>
>>> 2006/8/29, Andrew Zhang <zh...@gmail.com>:
>>> > On 8/28/06, Alexey Varlamov (JIRA) <ji...@apache.org> wrote:
>>> > >
>>> > > [classlib][net] flaw in setReuseAddrAndReusePort()
>>> > > --------------------------------------------------
>>> > >
>>> > > Key: HARMONY-1295
>>> > > URL:
>>> http://issues.apache.org/jira/browse/HARMONY-1295
>>> > > Project: Harmony
>>> > > Issue Type: Bug
>>> > > Components: Classlib
>>> > > Reporter: Alexey Varlamov
>>> > > Priority: Critical
>>> > >
>>> > >
>>> > > The tests.api.java.net.DatagramSocketTest test crashes on assert in
>>> DRLVM
>>> > > : Assertion `IsInstanceOf(env, obj,
>>> struct_Class_to_jclass(f->get_class()))'
>>> > > failed.
>>> > > The reason is that java.net.DatagramSocket.setReuseAddress(boolean)
>>> calls
>>> > > to
>>> > > PlainDatagramSocketImpl.setOption(int optID, Object val) with
>>> Boolean
>>> > > value, but underlying native code treats this value as Integer.
>>> > >
>>> > > Quickfix must be straightforward enough, but I inclined to blame
>>> the
>>> > > design of INetworkSystem.[set|get]SocketOption() as the actual
>>> root of
>>> the
>>> > > evil. The problem is that this API conceals the details of contract
>>> for
>>> > > particular options, and a client have to guess which type of
>>> value to
>>> pass.
>>> >
>>> >
>>> > I think it's "determine", not "guess". :)
>>> >
>>> > And the implementation is basically an ugly switch sorting
>>> requests out
>>> to
>>> > > setBoolSocketOption() or setIntegerSocketOption(), etc. Shouldn'te
>>> refactor
>>> > > this?
>>> >
>>> >
>>> > Any proposals? Thanks!
>>> >
>>> Of course, lots of them ;). Just to start:
>>>
>>> 1) Why at all we clone underlying OS interface to Java with extra
>>> mapping JAVASOCKOPT_* to HY_SO_*?
>>
>>
>> Totally agree with you. :) Current two option lists are duplicated. I
>> think
>> we'd better use one type defination.
>>
>> Let's have a clean API with
>>> specialized methods as XXXSocket classes have: setTTL(int) ,
>>> isReuseAddress() etc.
>>
>>
>> It's a design problem. The legacy design provides simple & unified
>> interface
>> to classlib, while your proposal seems more specific and direct for
>> classlib
>> API usage.
>>
>>
>> 2) At least we could provide more typified [s|g]etOption() methods,
>>> which take/return primitive values instead of wrappers. At first
>>> glance int and boolean would fit all needs, am I wrong? This way, type
>>> mismatch will be found immediately during compilation. Though this
>>> would not detect optid and value type disparity...
>>
>>
>> Agree. For this case, at least, we could use "booleanValue" for
>> Boolean and
>> "intValue" for Integer. :)
>>
>
> Interesting, deep look into code, I find JNI use pointer as the field
> ID, so field-ID of value in Boolean is the same as the one of Integer.
> In this way the code can get a value field of Integer out of a Boolean
> (C pointer is great! :) ). IMO, this is a mis-use but cause no error
> until a strict check is taken. :)
> This is a bug of native code, I agree we should use "booleanValue"
> instead of "intValue", may we raise a JIRA and fix?
Just go ahead and provide a patch for HARMONY-1295 to get this error
fixed, and go on discussing the design issue, I agree that there are
some things better to be improved.
>
>> --
>>> Regards,
>>> Alexey
>>>
>>> ---------------------------------------------------------------------
>>> Terms of use : http://incubator.apache.org/harmony/mailing.html
>>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
>>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>>>
>>>
>>
>>
>
>
--
Paulex Yang
China Software Development Lab
IBM
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: [jira] Created: (HARMONY-1295) [classlib][net] flaw in setReuseAddrAndReusePort()
Posted by "Jimmy, Jing Lv" <fi...@gmail.com>.
Andrew Zhang wrote:
> On 8/29/06, Alexey Varlamov <al...@gmail.com> wrote:
>>
>> 2006/8/29, Andrew Zhang <zh...@gmail.com>:
>> > On 8/28/06, Alexey Varlamov (JIRA) <ji...@apache.org> wrote:
>> > >
>> > > [classlib][net] flaw in setReuseAddrAndReusePort()
>> > > --------------------------------------------------
>> > >
>> > > Key: HARMONY-1295
>> > > URL:
>> http://issues.apache.org/jira/browse/HARMONY-1295
>> > > Project: Harmony
>> > > Issue Type: Bug
>> > > Components: Classlib
>> > > Reporter: Alexey Varlamov
>> > > Priority: Critical
>> > >
>> > >
>> > > The tests.api.java.net.DatagramSocketTest test crashes on assert in
>> DRLVM
>> > > : Assertion `IsInstanceOf(env, obj,
>> struct_Class_to_jclass(f->get_class()))'
>> > > failed.
>> > > The reason is that java.net.DatagramSocket.setReuseAddress(boolean)
>> calls
>> > > to
>> > > PlainDatagramSocketImpl.setOption(int optID, Object val) with Boolean
>> > > value, but underlying native code treats this value as Integer.
>> > >
>> > > Quickfix must be straightforward enough, but I inclined to blame the
>> > > design of INetworkSystem.[set|get]SocketOption() as the actual
>> root of
>> the
>> > > evil. The problem is that this API conceals the details of contract
>> for
>> > > particular options, and a client have to guess which type of value to
>> pass.
>> >
>> >
>> > I think it's "determine", not "guess". :)
>> >
>> > And the implementation is basically an ugly switch sorting requests out
>> to
>> > > setBoolSocketOption() or setIntegerSocketOption(), etc. Shouldn'te
>> refactor
>> > > this?
>> >
>> >
>> > Any proposals? Thanks!
>> >
>> Of course, lots of them ;). Just to start:
>>
>> 1) Why at all we clone underlying OS interface to Java with extra
>> mapping JAVASOCKOPT_* to HY_SO_*?
>
>
> Totally agree with you. :) Current two option lists are duplicated. I think
> we'd better use one type defination.
>
> Let's have a clean API with
>> specialized methods as XXXSocket classes have: setTTL(int) ,
>> isReuseAddress() etc.
>
>
> It's a design problem. The legacy design provides simple & unified
> interface
> to classlib, while your proposal seems more specific and direct for
> classlib
> API usage.
>
>
> 2) At least we could provide more typified [s|g]etOption() methods,
>> which take/return primitive values instead of wrappers. At first
>> glance int and boolean would fit all needs, am I wrong? This way, type
>> mismatch will be found immediately during compilation. Though this
>> would not detect optid and value type disparity...
>
>
> Agree. For this case, at least, we could use "booleanValue" for Boolean and
> "intValue" for Integer. :)
>
Interesting, deep look into code, I find JNI use pointer as the field
ID, so field-ID of value in Boolean is the same as the one of Integer.
In this way the code can get a value field of Integer out of a Boolean
(C pointer is great! :) ). IMO, this is a mis-use but cause no error
until a strict check is taken. :)
This is a bug of native code, I agree we should use "booleanValue"
instead of "intValue", may we raise a JIRA and fix?
> --
>> Regards,
>> Alexey
>>
>> ---------------------------------------------------------------------
>> Terms of use : http://incubator.apache.org/harmony/mailing.html
>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>>
>>
>
>
--
Best Regards!
Jimmy, Jing Lv
China Software Development Lab, IBM
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: [jira] Created: (HARMONY-1295) [classlib][net] flaw in setReuseAddrAndReusePort()
Posted by Andrew Zhang <zh...@gmail.com>.
On 8/29/06, Alexey Varlamov <al...@gmail.com> wrote:
>
> 2006/8/29, Andrew Zhang <zh...@gmail.com>:
> > On 8/28/06, Alexey Varlamov (JIRA) <ji...@apache.org> wrote:
> > >
> > > [classlib][net] flaw in setReuseAddrAndReusePort()
> > > --------------------------------------------------
> > >
> > > Key: HARMONY-1295
> > > URL: http://issues.apache.org/jira/browse/HARMONY-1295
> > > Project: Harmony
> > > Issue Type: Bug
> > > Components: Classlib
> > > Reporter: Alexey Varlamov
> > > Priority: Critical
> > >
> > >
> > > The tests.api.java.net.DatagramSocketTest test crashes on assert in
> DRLVM
> > > : Assertion `IsInstanceOf(env, obj,
> struct_Class_to_jclass(f->get_class()))'
> > > failed.
> > > The reason is that java.net.DatagramSocket.setReuseAddress(boolean)
> calls
> > > to
> > > PlainDatagramSocketImpl.setOption(int optID, Object val) with Boolean
> > > value, but underlying native code treats this value as Integer.
> > >
> > > Quickfix must be straightforward enough, but I inclined to blame the
> > > design of INetworkSystem.[set|get]SocketOption() as the actual root of
> the
> > > evil. The problem is that this API conceals the details of contract
> for
> > > particular options, and a client have to guess which type of value to
> pass.
> >
> >
> > I think it's "determine", not "guess". :)
> >
> > And the implementation is basically an ugly switch sorting requests out
> to
> > > setBoolSocketOption() or setIntegerSocketOption(), etc. Shouldn'te
> refactor
> > > this?
> >
> >
> > Any proposals? Thanks!
> >
> Of course, lots of them ;). Just to start:
>
> 1) Why at all we clone underlying OS interface to Java with extra
> mapping JAVASOCKOPT_* to HY_SO_*?
Totally agree with you. :) Current two option lists are duplicated. I think
we'd better use one type defination.
Let's have a clean API with
> specialized methods as XXXSocket classes have: setTTL(int) ,
> isReuseAddress() etc.
It's a design problem. The legacy design provides simple & unified interface
to classlib, while your proposal seems more specific and direct for classlib
API usage.
2) At least we could provide more typified [s|g]etOption() methods,
> which take/return primitive values instead of wrappers. At first
> glance int and boolean would fit all needs, am I wrong? This way, type
> mismatch will be found immediately during compilation. Though this
> would not detect optid and value type disparity...
Agree. For this case, at least, we could use "booleanValue" for Boolean and
"intValue" for Integer. :)
--
> Regards,
> Alexey
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>
--
Andrew Zhang
China Software Development Lab, IBM
Re: [jira] Created: (HARMONY-1295) [classlib][net] flaw in setReuseAddrAndReusePort()
Posted by Alexey Varlamov <al...@gmail.com>.
2006/8/29, Andrew Zhang <zh...@gmail.com>:
> On 8/28/06, Alexey Varlamov (JIRA) <ji...@apache.org> wrote:
> >
> > [classlib][net] flaw in setReuseAddrAndReusePort()
> > --------------------------------------------------
> >
> > Key: HARMONY-1295
> > URL: http://issues.apache.org/jira/browse/HARMONY-1295
> > Project: Harmony
> > Issue Type: Bug
> > Components: Classlib
> > Reporter: Alexey Varlamov
> > Priority: Critical
> >
> >
> > The tests.api.java.net.DatagramSocketTest test crashes on assert in DRLVM
> > : Assertion `IsInstanceOf(env, obj, struct_Class_to_jclass(f->get_class()))'
> > failed.
> > The reason is that java.net.DatagramSocket.setReuseAddress(boolean) calls
> > to
> > PlainDatagramSocketImpl.setOption(int optID, Object val) with Boolean
> > value, but underlying native code treats this value as Integer.
> >
> > Quickfix must be straightforward enough, but I inclined to blame the
> > design of INetworkSystem.[set|get]SocketOption() as the actual root of the
> > evil. The problem is that this API conceals the details of contract for
> > particular options, and a client have to guess which type of value to pass.
>
>
> I think it's "determine", not "guess". :)
>
> And the implementation is basically an ugly switch sorting requests out to
> > setBoolSocketOption() or setIntegerSocketOption(), etc. Shouldn'te refactor
> > this?
>
>
> Any proposals? Thanks!
>
Of course, lots of them ;). Just to start:
1) Why at all we clone underlying OS interface to Java with extra
mapping JAVASOCKOPT_* to HY_SO_*? Let's have a clean API with
specialized methods as XXXSocket classes have: setTTL(int) ,
isReuseAddress() etc.
2) At least we could provide more typified [s|g]etOption() methods,
which take/return primitive values instead of wrappers. At first
glance int and boolean would fit all needs, am I wrong? This way, type
mismatch will be found immediately during compilation. Though this
would not detect optid and value type disparity...
--
Regards,
Alexey
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org