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/09/14 12:16:35 UTC

[classlib][net] The old fix is not complete (was Re: [jira] Updated: (HARMONY-1117) [classlib][net] Harmony implementation of SocketImpl.getFileDescriptor() return real object, but RI return null)

 I don't think the fix is complete. The patch only fixed reported bug, but
introduces new severe bugs.

Because fd would never be initialized after applying the patch. Running all
tests would also fail with crash.

After deleting initialization code in SocketImpl.java, we have to initialize
SocketImpl.fd elsewhere. I suggest add the code into
PlainSocketImpl.createmethod.  Am I right?

If no one objects, I'll apply a patch to fix this jira. Thanks!

On 8/9/06, Igor V. Stolyarov (JIRA) <ji...@apache.org> wrote:
>
>     [ http://issues.apache.org/jira/browse/HARMONY-1117?page=all ]
>
> Igor V. Stolyarov updated HARMONY-1117:
> ---------------------------------------
>
>    Attachment: Harmony-1117.patch
>
> Fix attached
>
> > [classlib][net] Harmony implementation of SocketImpl.getFileDescriptor()
> return real object, but RI return null
> >
> ---------------------------------------------------------------------------------------------------------------
> >
> >                 Key: HARMONY-1117
> >                 URL: http://issues.apache.org/jira/browse/HARMONY-1117
> >             Project: Harmony
> >          Issue Type: Bug
> >            Reporter: Igor V. Stolyarov
> >         Attachments: Harmony-1117.patch
> >
> >
> >  Harmony implementation of SocketImpl.getFileDescriptor() return real
> object, but RI return null
> >
> Test---------------------------------------------------------------------------------------------------
> > import java.io.*;
> > import java.net.*;
> > public class Test {
> >     public static void main(String[] args) {
> >        try {
> >             TestSocketImpl testSocketImpl = new TestSocketImpl();
> >             System.out.println("res="+testSocketImpl.getFileDescriptor()
> );
> >        } catch (Exception e) {
> >            e.printStackTrace();
> >        }
> >     }
> > }
> > class TestSocketImpl extends SocketImpl {
> >     public FileDescriptor getFileDescriptor(){
> >         return super.getFileDescriptor();
> >     }
> >     protected void create(boolean arg0) throws IOException {}
> >     protected void connect(String arg0, int arg1) throws IOException {}
> >     protected void connect(InetAddress arg0, int arg1) throws
> IOException {}
> >     protected void connect(SocketAddress arg0, int arg1) throws
> IOException {}
> >     protected void bind(InetAddress arg0, int arg1) throws IOException{}
> >     protected void listen(int arg0) throws IOException {}
> >     protected void accept(SocketImpl arg0) throws IOException {}
> >     protected InputStream getInputStream() throws IOException {
> >         return null;
> >     }
> >     protected OutputStream getOutputStream() throws IOException {
> >         return null;
> >     }
> >     protected int available() throws IOException {
> >         return 0;
> >     }
> >     protected void close() throws IOException {}
> >     protected void sendUrgentData(int arg0) throws IOException {}
> >     public void setOption(int arg0, Object arg1) throws
> SocketException{}
> >     public Object getOption(int arg0) throws SocketException {
> >         return null;
> >     }
> > }
> >
> Output------------------------------------------------------------------------------------------------
> > Harmony:
> > java version 1.5 (subset)
> > (c) Copyright 1991, 2006 The Apache Software Foundation or its
> licensors, as app
> > licable.
> > res=java.io.FileDescriptor@6460646
> > JRockit:
> > java version "1.5.0"
> > Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0-b64)
> > BEA WebLogic JRockit(R) (build dra-38972-20041208-2001-win-ia32,
> R25.0.0-75, GC:
> >  System optimized over throughput (initial strategy singleparpar))
> > res=null
>
> --
> 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: [classlib][net] The old fix is not complete (was Re: [jira] Updated: (HARMONY-1117) [classlib][net] Harmony implementation of SocketImpl.getFileDescriptor() return real object, but RI return null)

Posted by Andrew Zhang <zh...@gmail.com>.
Patch was updated.

Robert Hu, I agree with you that re-new fd in close method looks really
strange. But for quick fix, I put the fd initialization code in the
subclass. After all, lots of code in luni/nio modules are involved in fd
issue.

I'll study whether it's feasible to remove "fd initliazation" code from
close method and put the code in "create" method. I also noticed that fd
initialization is a bit random. It appears in many places in luni/nio.  I
may fix the problem with a seperate jira if it's feasible.

Thanks!


On 9/15/06, Andrew Zhang <zh...@gmail.com> wrote:
>
>
>  On 9/14/06, Alexey Petrenko <al...@gmail.com> wrote:
> >
> > Yes, it looks like fd field should be initialized by subclass.
> >
> > Andrew, are you going to suggest a new patch?
>
>
>  Yes. :) But I'll take vacation today, and should upload a new patch next
> Monday. :-)
>
> SY, Alexey
> >
> > 2006/9/14, Jimmy, Jing Lv <firepure@gmail.com >:
> > > Andrew Zhang wrote:
> > > > I don't think the fix is complete. The patch only fixed reported
> > bug, but
> > > > introduces new severe bugs.
> > > >
> > > > Because fd would never be initialized after applying the patch.
> > Running all
> > > > tests would also fail with crash.
> > > >
> > >
> > > Yes, fd without initialized may cause a tragedy, all network functions
> > > relay on this.
> > >
> > > > After deleting initialization code in SocketImpl.java, we have to
> > > > initialize
> > > > SocketImpl.fd elsewhere. I suggest add the code into
> > > > PlainSocketImpl.createmethod.  Am I right?
> > > >
> > >
> > > I believe this may be correct. Or we can add a constructor in
> > > PlainSocketImpl, initial fd there.
> > >
> > > > If no one objects, I'll apply a patch to fix this jira. Thanks!
> > > >
> > > > On 8/9/06, Igor V. Stolyarov (JIRA) < jira@apache.org> wrote:
> > > >>
> > > >>     [ http://issues.apache.org/jira/browse/HARMONY-1117?page=all ]
> > > >>
> > > >> Igor V. Stolyarov updated HARMONY-1117:
> > > >> ---------------------------------------
> > > >>
> > > >>    Attachment: Harmony-1117.patch
> > > >>
> > > >> Fix attached
> > > >>
> > > >> > [classlib][net] Harmony implementation of
> > > >> SocketImpl.getFileDescriptor()
> > > >> return real object, but RI return null
> > > >> >
> > > >>
> > ---------------------------------------------------------------------------------------------------------------
> >
> > > >>
> > > >> >
> > > >> >                 Key: HARMONY-1117
> > > >> >                 URL: http://issues.apache.org/jira/browse/HARMONY-1117
> >
> > > >> >             Project: Harmony
> > > >> >          Issue Type: Bug
> > > >> >            Reporter: Igor V. Stolyarov
> > > >> >         Attachments: Harmony-1117.patch
> > > >> >
> > > >> >
> > > >> >  Harmony implementation of SocketImpl.getFileDescriptor() return
> > real
> > > >> object, but RI return null
> > > >> >
> > > >>
> > Test---------------------------------------------------------------------------------------------------
> >
> > > >>
> > > >> > import java.io.*;
> > > >> > import java.net.*;
> > > >> > public class Test {
> > > >> >     public static void main(String[] args) {
> > > >> >        try {
> > > >> >             TestSocketImpl testSocketImpl = new TestSocketImpl();
> > > >> >
> > > >> System.out.println("res="+testSocketImpl.getFileDescriptor()
> > > >> );
> > > >> >        } catch (Exception e) {
> > > >> >            e.printStackTrace();
> > > >> >        }
> > > >> >     }
> > > >> > }
> > > >> > class TestSocketImpl extends SocketImpl {
> > > >> >     public FileDescriptor getFileDescriptor(){
> > > >> >         return super.getFileDescriptor();
> > > >> >     }
> > > >> >     protected void create(boolean arg0) throws IOException {}
> > > >> >     protected void connect(String arg0, int arg1) throws
> > IOException {}
> > > >> >     protected void connect(InetAddress arg0, int arg1) throws
> > > >> IOException {}
> > > >> >     protected void connect(SocketAddress arg0, int arg1) throws
> > > >> IOException {}
> > > >> >     protected void bind(InetAddress arg0, int arg1) throws
> > > >> IOException{}
> > > >> >     protected void listen(int arg0) throws IOException {}
> > > >> >     protected void accept(SocketImpl arg0) throws IOException {}
> > > >> >     protected InputStream getInputStream() throws IOException {
> > > >> >         return null;
> > > >> >     }
> > > >> >     protected OutputStream getOutputStream() throws IOException {
> > > >> >         return null;
> > > >> >     }
> > > >> >     protected int available() throws IOException {
> > > >> >         return 0;
> > > >> >     }
> > > >> >     protected void close() throws IOException {}
> > > >> >     protected void sendUrgentData(int arg0) throws IOException {}
> >
> > > >> >     public void setOption(int arg0, Object arg1) throws
> > > >> SocketException{}
> > > >> >     public Object getOption(int arg0) throws SocketException {
> > > >> >         return null;
> > > >> >     }
> > > >> > }
> > > >> >
> > > >>
> > Output------------------------------------------------------------------------------------------------
> > > >>
> > > >> > Harmony:
> > > >> > java version 1.5 (subset)
> > > >> > (c) Copyright 1991, 2006 The Apache Software Foundation or its
> > > >> licensors, as app
> > > >> > licable.
> > > >> > res= java.io.FileDescriptor@6460646
> > > >> > JRockit:
> > > >> > java version "1.5.0"
> > > >> > Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0-b64
> > )
> > > >> > BEA WebLogic JRockit(R) (build dra-38972-20041208-2001-win-ia32,
> > > >> R25.0.0-75, GC:
> > > >> >  System optimized over throughput (initial strategy
> > singleparpar))
> > > >> > res=null
> > > >>
> > > >> --
> > > >> 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
> > > >>
> > > >>
> > > >>
> > > >
> > > >
> > >
> > >
> > > --
> > >
> > > 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
> > >
> > >
> >
> >
> > --
> > Alexey A. Petrenko
> > Intel Middleware Products Division
> >
> > ---------------------------------------------------------------------
> > 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
>



-- 
Andrew Zhang
China Software Development Lab, IBM

Re: [classlib][net] The old fix is not complete (was Re: [jira] Updated: (HARMONY-1117) [classlib][net] Harmony implementation of SocketImpl.getFileDescriptor() return real object, but RI return null)

Posted by Andrew Zhang <zh...@gmail.com>.
On 9/14/06, Alexey Petrenko <al...@gmail.com> wrote:
>
> Yes, it looks like fd field should be initialized by subclass.
>
> Andrew, are you going to suggest a new patch?


Yes. :) But I'll take vacation today, and should upload a new patch next
Monday. :-)

SY, Alexey
>
> 2006/9/14, Jimmy, Jing Lv <fi...@gmail.com>:
> > Andrew Zhang wrote:
> > > I don't think the fix is complete. The patch only fixed reported bug,
> but
> > > introduces new severe bugs.
> > >
> > > Because fd would never be initialized after applying the patch.
> Running all
> > > tests would also fail with crash.
> > >
> >
> > Yes, fd without initialized may cause a tragedy, all network functions
> > relay on this.
> >
> > > After deleting initialization code in SocketImpl.java, we have to
> > > initialize
> > > SocketImpl.fd elsewhere. I suggest add the code into
> > > PlainSocketImpl.createmethod.  Am I right?
> > >
> >
> > I believe this may be correct. Or we can add a constructor in
> > PlainSocketImpl, initial fd there.
> >
> > > If no one objects, I'll apply a patch to fix this jira. Thanks!
> > >
> > > On 8/9/06, Igor V. Stolyarov (JIRA) <ji...@apache.org> wrote:
> > >>
> > >>     [ http://issues.apache.org/jira/browse/HARMONY-1117?page=all ]
> > >>
> > >> Igor V. Stolyarov updated HARMONY-1117:
> > >> ---------------------------------------
> > >>
> > >>    Attachment: Harmony-1117.patch
> > >>
> > >> Fix attached
> > >>
> > >> > [classlib][net] Harmony implementation of
> > >> SocketImpl.getFileDescriptor()
> > >> return real object, but RI return null
> > >> >
> > >>
> ---------------------------------------------------------------------------------------------------------------
> > >>
> > >> >
> > >> >                 Key: HARMONY-1117
> > >> >                 URL:
> http://issues.apache.org/jira/browse/HARMONY-1117
> > >> >             Project: Harmony
> > >> >          Issue Type: Bug
> > >> >            Reporter: Igor V. Stolyarov
> > >> >         Attachments: Harmony-1117.patch
> > >> >
> > >> >
> > >> >  Harmony implementation of SocketImpl.getFileDescriptor() return
> real
> > >> object, but RI return null
> > >> >
> > >>
> Test---------------------------------------------------------------------------------------------------
> > >>
> > >> > import java.io.*;
> > >> > import java.net.*;
> > >> > public class Test {
> > >> >     public static void main(String[] args) {
> > >> >        try {
> > >> >             TestSocketImpl testSocketImpl = new TestSocketImpl();
> > >> >
> > >> System.out.println("res="+testSocketImpl.getFileDescriptor()
> > >> );
> > >> >        } catch (Exception e) {
> > >> >            e.printStackTrace();
> > >> >        }
> > >> >     }
> > >> > }
> > >> > class TestSocketImpl extends SocketImpl {
> > >> >     public FileDescriptor getFileDescriptor(){
> > >> >         return super.getFileDescriptor();
> > >> >     }
> > >> >     protected void create(boolean arg0) throws IOException {}
> > >> >     protected void connect(String arg0, int arg1) throws
> IOException {}
> > >> >     protected void connect(InetAddress arg0, int arg1) throws
> > >> IOException {}
> > >> >     protected void connect(SocketAddress arg0, int arg1) throws
> > >> IOException {}
> > >> >     protected void bind(InetAddress arg0, int arg1) throws
> > >> IOException{}
> > >> >     protected void listen(int arg0) throws IOException {}
> > >> >     protected void accept(SocketImpl arg0) throws IOException {}
> > >> >     protected InputStream getInputStream() throws IOException {
> > >> >         return null;
> > >> >     }
> > >> >     protected OutputStream getOutputStream() throws IOException {
> > >> >         return null;
> > >> >     }
> > >> >     protected int available() throws IOException {
> > >> >         return 0;
> > >> >     }
> > >> >     protected void close() throws IOException {}
> > >> >     protected void sendUrgentData(int arg0) throws IOException {}
> > >> >     public void setOption(int arg0, Object arg1) throws
> > >> SocketException{}
> > >> >     public Object getOption(int arg0) throws SocketException {
> > >> >         return null;
> > >> >     }
> > >> > }
> > >> >
> > >>
> Output------------------------------------------------------------------------------------------------
> > >>
> > >> > Harmony:
> > >> > java version 1.5 (subset)
> > >> > (c) Copyright 1991, 2006 The Apache Software Foundation or its
> > >> licensors, as app
> > >> > licable.
> > >> > res=java.io.FileDescriptor@6460646
> > >> > JRockit:
> > >> > java version "1.5.0"
> > >> > Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0-b64)
> > >> > BEA WebLogic JRockit(R) (build dra-38972-20041208-2001-win-ia32,
> > >> R25.0.0-75, GC:
> > >> >  System optimized over throughput (initial strategy singleparpar))
> > >> > res=null
> > >>
> > >> --
> > >> 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
> > >>
> > >>
> > >>
> > >
> > >
> >
> >
> > --
> >
> > 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
> >
> >
>
>
> --
> Alexey A. Petrenko
> Intel Middleware Products Division
>
> ---------------------------------------------------------------------
> 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: [classlib][net] The old fix is not complete (was Re: [jira] Updated: (HARMONY-1117) [classlib][net] Harmony implementation of SocketImpl.getFileDescriptor() return real object, but RI return null)

Posted by Alexey Petrenko <al...@gmail.com>.
Yes, it looks like fd field should be initialized by subclass.

Andrew, are you going to suggest a new patch?

SY, Alexey

2006/9/14, Jimmy, Jing Lv <fi...@gmail.com>:
> Andrew Zhang wrote:
> > I don't think the fix is complete. The patch only fixed reported bug, but
> > introduces new severe bugs.
> >
> > Because fd would never be initialized after applying the patch. Running all
> > tests would also fail with crash.
> >
>
> Yes, fd without initialized may cause a tragedy, all network functions
> relay on this.
>
> > After deleting initialization code in SocketImpl.java, we have to
> > initialize
> > SocketImpl.fd elsewhere. I suggest add the code into
> > PlainSocketImpl.createmethod.  Am I right?
> >
>
> I believe this may be correct. Or we can add a constructor in
> PlainSocketImpl, initial fd there.
>
> > If no one objects, I'll apply a patch to fix this jira. Thanks!
> >
> > On 8/9/06, Igor V. Stolyarov (JIRA) <ji...@apache.org> wrote:
> >>
> >>     [ http://issues.apache.org/jira/browse/HARMONY-1117?page=all ]
> >>
> >> Igor V. Stolyarov updated HARMONY-1117:
> >> ---------------------------------------
> >>
> >>    Attachment: Harmony-1117.patch
> >>
> >> Fix attached
> >>
> >> > [classlib][net] Harmony implementation of
> >> SocketImpl.getFileDescriptor()
> >> return real object, but RI return null
> >> >
> >> ---------------------------------------------------------------------------------------------------------------
> >>
> >> >
> >> >                 Key: HARMONY-1117
> >> >                 URL: http://issues.apache.org/jira/browse/HARMONY-1117
> >> >             Project: Harmony
> >> >          Issue Type: Bug
> >> >            Reporter: Igor V. Stolyarov
> >> >         Attachments: Harmony-1117.patch
> >> >
> >> >
> >> >  Harmony implementation of SocketImpl.getFileDescriptor() return real
> >> object, but RI return null
> >> >
> >> Test---------------------------------------------------------------------------------------------------
> >>
> >> > import java.io.*;
> >> > import java.net.*;
> >> > public class Test {
> >> >     public static void main(String[] args) {
> >> >        try {
> >> >             TestSocketImpl testSocketImpl = new TestSocketImpl();
> >> >
> >> System.out.println("res="+testSocketImpl.getFileDescriptor()
> >> );
> >> >        } catch (Exception e) {
> >> >            e.printStackTrace();
> >> >        }
> >> >     }
> >> > }
> >> > class TestSocketImpl extends SocketImpl {
> >> >     public FileDescriptor getFileDescriptor(){
> >> >         return super.getFileDescriptor();
> >> >     }
> >> >     protected void create(boolean arg0) throws IOException {}
> >> >     protected void connect(String arg0, int arg1) throws IOException {}
> >> >     protected void connect(InetAddress arg0, int arg1) throws
> >> IOException {}
> >> >     protected void connect(SocketAddress arg0, int arg1) throws
> >> IOException {}
> >> >     protected void bind(InetAddress arg0, int arg1) throws
> >> IOException{}
> >> >     protected void listen(int arg0) throws IOException {}
> >> >     protected void accept(SocketImpl arg0) throws IOException {}
> >> >     protected InputStream getInputStream() throws IOException {
> >> >         return null;
> >> >     }
> >> >     protected OutputStream getOutputStream() throws IOException {
> >> >         return null;
> >> >     }
> >> >     protected int available() throws IOException {
> >> >         return 0;
> >> >     }
> >> >     protected void close() throws IOException {}
> >> >     protected void sendUrgentData(int arg0) throws IOException {}
> >> >     public void setOption(int arg0, Object arg1) throws
> >> SocketException{}
> >> >     public Object getOption(int arg0) throws SocketException {
> >> >         return null;
> >> >     }
> >> > }
> >> >
> >> Output------------------------------------------------------------------------------------------------
> >>
> >> > Harmony:
> >> > java version 1.5 (subset)
> >> > (c) Copyright 1991, 2006 The Apache Software Foundation or its
> >> licensors, as app
> >> > licable.
> >> > res=java.io.FileDescriptor@6460646
> >> > JRockit:
> >> > java version "1.5.0"
> >> > Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0-b64)
> >> > BEA WebLogic JRockit(R) (build dra-38972-20041208-2001-win-ia32,
> >> R25.0.0-75, GC:
> >> >  System optimized over throughput (initial strategy singleparpar))
> >> > res=null
> >>
> >> --
> >> 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
> >>
> >>
> >>
> >
> >
>
>
> --
>
> 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
>
>


-- 
Alexey A. Petrenko
Intel Middleware Products Division

---------------------------------------------------------------------
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: [classlib][net] The old fix is not complete (was Re: [jira] Updated: (HARMONY-1117) [classlib][net] Harmony implementation of SocketImpl.getFileDescriptor() return real object, but RI return null)

Posted by Rui Hu <ro...@gmail.com>.
On 9/15/06, LvJimmy,Jing <fi...@gmail.com> wrote:
>
> <snip>
>
>
> Ah yes, I remember this, this caused a bug in interaction with NIO
> channels
> :)
>
> However, fd-initialization  and socket-creation are two different things.
> fd-initialization means only "new FileDescriptor()", but create socket
> means
> create a socket in native code.


Yes, but we must initialize the fd whenever fd is not valid and needed, the
create() method is the good place to check and do it.

fd-initialization in Constructor is at least good:
> 1. fix the bug, but not break current correct logic at all, so it may pass
> all the test


Oh, I see, you're right. But I found that in current implementation there is
a "fd = new FileDescriptor();" in PlainSocketImpl.close()  method. IMO, it
is strange. And it's the reason why your constructor solution is OK.
If we set fd to null in the close method, we must add "new FileDescriptor"
in creat method. It is more reasonable, but will cost more effort to
refactor current implementation.
So I agree with you on this occasion.  :)

2. may not new another fd, so save a little memory ;)


However I have no objection on Andrew's idea :)
>
> <snip>
>
>
>
> --
>
> Best Regards!
>
> Jimmy, Jing Lv
> China Software Development Lab, IBM
>
>


-- 
Robert Hu
China Software Development Lab, IBM

Re: [classlib][net] The old fix is not complete (was Re: [jira] Updated: (HARMONY-1117) [classlib][net] Harmony implementation of SocketImpl.getFileDescriptor() return real object, but RI return null)

Posted by LvJimmy,Jing <fi...@gmail.com>.
2006/9/14, Rui Hu <ro...@gmail.com>:
>
> On 9/14/06, Jimmy, Jing Lv <fi...@gmail.com> wrote:
> >
> > Andrew Zhang wrote:
> > > I don't think the fix is complete. The patch only fixed reported bug,
> > but
> > > introduces new severe bugs.
> > >
> > > Because fd would never be initialized after applying the patch.
> Running
> > all
> > > tests would also fail with crash.
> > >
> >
> > Yes, fd without initialized may cause a tragedy, all network functions
> > relay on this.
> >
> > > After deleting initialization code in SocketImpl.java, we have to
> > > initialize
> > > SocketImpl.fd elsewhere. I suggest add the code into
> > > PlainSocketImpl.createmethod.  Am I right?
> > >
> >
> > I believe this may be correct. Or we can add a constructor in
> > PlainSocketImpl, initial fd there.
>
>
> IMHO, Andrew's suggestion is better.
> The fd should be initialized not only when constructor is called, but also
> in some other occasions.
> e.g. PlainSocketImpl.create is called by checkClosedAndCreate(boolean) of
> Socket/ServerSocket class, and widely called by other
> methods(bind,connect...etc) indirectly, when a Socket/ServerSocket try to
> check status(open/closed) and re-create its netimpl.




Ah yes, I remember this, this caused a bug in interaction with NIO channels
:)

However, fd-initialization  and socket-creation are two different things.
fd-initialization means only "new FileDescriptor()", but create socket means
create a socket in native code.

fd-initialization in Constructor is at least good:
1. fix the bug, but not break current correct logic at all, so it may pass
all the test
2. may not new another fd, so save a little memory ;)

However I have no objection on Andrew's idea :)

<snip>



-- 

Best Regards!

Jimmy, Jing Lv
China Software Development Lab, IBM

Re: [classlib][net] The old fix is not complete (was Re: [jira] Updated: (HARMONY-1117) [classlib][net] Harmony implementation of SocketImpl.getFileDescriptor() return real object, but RI return null)

Posted by Rui Hu <ro...@gmail.com>.
On 9/14/06, Jimmy, Jing Lv <fi...@gmail.com> wrote:
>
> Andrew Zhang wrote:
> > I don't think the fix is complete. The patch only fixed reported bug,
> but
> > introduces new severe bugs.
> >
> > Because fd would never be initialized after applying the patch. Running
> all
> > tests would also fail with crash.
> >
>
> Yes, fd without initialized may cause a tragedy, all network functions
> relay on this.
>
> > After deleting initialization code in SocketImpl.java, we have to
> > initialize
> > SocketImpl.fd elsewhere. I suggest add the code into
> > PlainSocketImpl.createmethod.  Am I right?
> >
>
> I believe this may be correct. Or we can add a constructor in
> PlainSocketImpl, initial fd there.


IMHO, Andrew's suggestion is better.
The fd should be initialized not only when constructor is called, but also
in some other occasions.
e.g. PlainSocketImpl.create is called by checkClosedAndCreate(boolean) of
Socket/ServerSocket class, and widely called by other
methods(bind,connect...etc) indirectly, when a Socket/ServerSocket try to
check status(open/closed) and re-create its netimpl.

> If no one objects, I'll apply a patch to fix this jira. Thanks!
> >
> > On 8/9/06, Igor V. Stolyarov (JIRA) <ji...@apache.org> wrote:
> >>
> >>     [ http://issues.apache.org/jira/browse/HARMONY-1117?page=all ]
> >>
> >> Igor V. Stolyarov updated HARMONY-1117:
> >> ---------------------------------------
> >>
> >>    Attachment: Harmony-1117.patch
> >>
> >> Fix attached
> >>
> >> > [classlib][net] Harmony implementation of
> >> SocketImpl.getFileDescriptor()
> >> return real object, but RI return null
> >> >
> >>
> ---------------------------------------------------------------------------------------------------------------
> >>
> >> >
> >> >                 Key: HARMONY-1117
> >> >                 URL:
> http://issues.apache.org/jira/browse/HARMONY-1117
> >> >             Project: Harmony
> >> >          Issue Type: Bug
> >> >            Reporter: Igor V. Stolyarov
> >> >         Attachments: Harmony-1117.patch
> >> >
> >> >
> >> >  Harmony implementation of SocketImpl.getFileDescriptor() return real
> >> object, but RI return null
> >> >
> >>
> Test---------------------------------------------------------------------------------------------------
> >>
> >> > import java.io.*;
> >> > import java.net.*;
> >> > public class Test {
> >> >     public static void main(String[] args) {
> >> >        try {
> >> >             TestSocketImpl testSocketImpl = new TestSocketImpl();
> >> >
> >> System.out.println("res="+testSocketImpl.getFileDescriptor()
> >> );
> >> >        } catch (Exception e) {
> >> >            e.printStackTrace();
> >> >        }
> >> >     }
> >> > }
> >> > class TestSocketImpl extends SocketImpl {
> >> >     public FileDescriptor getFileDescriptor(){
> >> >         return super.getFileDescriptor();
> >> >     }
> >> >     protected void create(boolean arg0) throws IOException {}
> >> >     protected void connect(String arg0, int arg1) throws IOException
> {}
> >> >     protected void connect(InetAddress arg0, int arg1) throws
> >> IOException {}
> >> >     protected void connect(SocketAddress arg0, int arg1) throws
> >> IOException {}
> >> >     protected void bind(InetAddress arg0, int arg1) throws
> >> IOException{}
> >> >     protected void listen(int arg0) throws IOException {}
> >> >     protected void accept(SocketImpl arg0) throws IOException {}
> >> >     protected InputStream getInputStream() throws IOException {
> >> >         return null;
> >> >     }
> >> >     protected OutputStream getOutputStream() throws IOException {
> >> >         return null;
> >> >     }
> >> >     protected int available() throws IOException {
> >> >         return 0;
> >> >     }
> >> >     protected void close() throws IOException {}
> >> >     protected void sendUrgentData(int arg0) throws IOException {}
> >> >     public void setOption(int arg0, Object arg1) throws
> >> SocketException{}
> >> >     public Object getOption(int arg0) throws SocketException {
> >> >         return null;
> >> >     }
> >> > }
> >> >
> >>
> Output------------------------------------------------------------------------------------------------
> >>
> >> > Harmony:
> >> > java version 1.5 (subset)
> >> > (c) Copyright 1991, 2006 The Apache Software Foundation or its
> >> licensors, as app
> >> > licable.
> >> > res=java.io.FileDescriptor@6460646
> >> > JRockit:
> >> > java version "1.5.0"
> >> > Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0-b64)
> >> > BEA WebLogic JRockit(R) (build dra-38972-20041208-2001-win-ia32,
> >> R25.0.0-75, GC:
> >> >  System optimized over throughput (initial strategy singleparpar))
> >> > res=null
> >>
> >> --
> >> 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
> >>
> >>
> >>
> >
> >
>
>
> --
>
> 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
>
>


-- 
Robert Hu
China Software Development Lab, IBM

Re: [classlib][net] The old fix is not complete (was Re: [jira] Updated: (HARMONY-1117) [classlib][net] Harmony implementation of SocketImpl.getFileDescriptor() return real object, but RI return null)

Posted by "Jimmy, Jing Lv" <fi...@gmail.com>.
Andrew Zhang wrote:
> I don't think the fix is complete. The patch only fixed reported bug, but
> introduces new severe bugs.
> 
> Because fd would never be initialized after applying the patch. Running all
> tests would also fail with crash.
> 

Yes, fd without initialized may cause a tragedy, all network functions 
relay on this.

> After deleting initialization code in SocketImpl.java, we have to 
> initialize
> SocketImpl.fd elsewhere. I suggest add the code into
> PlainSocketImpl.createmethod.  Am I right?
> 

I believe this may be correct. Or we can add a constructor in 
PlainSocketImpl, initial fd there.

> If no one objects, I'll apply a patch to fix this jira. Thanks!
> 
> On 8/9/06, Igor V. Stolyarov (JIRA) <ji...@apache.org> wrote:
>>
>>     [ http://issues.apache.org/jira/browse/HARMONY-1117?page=all ]
>>
>> Igor V. Stolyarov updated HARMONY-1117:
>> ---------------------------------------
>>
>>    Attachment: Harmony-1117.patch
>>
>> Fix attached
>>
>> > [classlib][net] Harmony implementation of 
>> SocketImpl.getFileDescriptor()
>> return real object, but RI return null
>> >
>> --------------------------------------------------------------------------------------------------------------- 
>>
>> >
>> >                 Key: HARMONY-1117
>> >                 URL: http://issues.apache.org/jira/browse/HARMONY-1117
>> >             Project: Harmony
>> >          Issue Type: Bug
>> >            Reporter: Igor V. Stolyarov
>> >         Attachments: Harmony-1117.patch
>> >
>> >
>> >  Harmony implementation of SocketImpl.getFileDescriptor() return real
>> object, but RI return null
>> >
>> Test--------------------------------------------------------------------------------------------------- 
>>
>> > import java.io.*;
>> > import java.net.*;
>> > public class Test {
>> >     public static void main(String[] args) {
>> >        try {
>> >             TestSocketImpl testSocketImpl = new TestSocketImpl();
>> >             
>> System.out.println("res="+testSocketImpl.getFileDescriptor()
>> );
>> >        } catch (Exception e) {
>> >            e.printStackTrace();
>> >        }
>> >     }
>> > }
>> > class TestSocketImpl extends SocketImpl {
>> >     public FileDescriptor getFileDescriptor(){
>> >         return super.getFileDescriptor();
>> >     }
>> >     protected void create(boolean arg0) throws IOException {}
>> >     protected void connect(String arg0, int arg1) throws IOException {}
>> >     protected void connect(InetAddress arg0, int arg1) throws
>> IOException {}
>> >     protected void connect(SocketAddress arg0, int arg1) throws
>> IOException {}
>> >     protected void bind(InetAddress arg0, int arg1) throws 
>> IOException{}
>> >     protected void listen(int arg0) throws IOException {}
>> >     protected void accept(SocketImpl arg0) throws IOException {}
>> >     protected InputStream getInputStream() throws IOException {
>> >         return null;
>> >     }
>> >     protected OutputStream getOutputStream() throws IOException {
>> >         return null;
>> >     }
>> >     protected int available() throws IOException {
>> >         return 0;
>> >     }
>> >     protected void close() throws IOException {}
>> >     protected void sendUrgentData(int arg0) throws IOException {}
>> >     public void setOption(int arg0, Object arg1) throws
>> SocketException{}
>> >     public Object getOption(int arg0) throws SocketException {
>> >         return null;
>> >     }
>> > }
>> >
>> Output------------------------------------------------------------------------------------------------ 
>>
>> > Harmony:
>> > java version 1.5 (subset)
>> > (c) Copyright 1991, 2006 The Apache Software Foundation or its
>> licensors, as app
>> > licable.
>> > res=java.io.FileDescriptor@6460646
>> > JRockit:
>> > java version "1.5.0"
>> > Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0-b64)
>> > BEA WebLogic JRockit(R) (build dra-38972-20041208-2001-win-ia32,
>> R25.0.0-75, GC:
>> >  System optimized over throughput (initial strategy singleparpar))
>> > res=null
>>
>> -- 
>> 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
>>
>>
>>
> 
> 


-- 

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