Re: [cp-patches] RFC: new VM interface for Socket impls

[Guilhem Lavaux]

Hi Roman,

I have a few problem concerning the VM interface.

1) accept() & close() may be called at the same time. Calling close() in 
another thread should stop the accept() call this is perfectly valid and 
implemented in kaffe/gcj/jdk. I know that in general we must authorize 
the user to use the same socket for different operations in different 
threads at the same time because that is authorized by the JDK.

2) I have a comment concerning the use of SO_TIMEOUT. At least on Unix 
this option is not generally implemented correctly (consistently ?) by 
the OS. So generally implementations relies on select/poll to really 
have operations which have a time out. Either the native will have to 
take care of this either we change this default implementation (I have 
seen it in connect)

Regards,

Guilhem.

Roman Kennke wrote:
> Hi there,
>
> I have put together a new proposal for the VM interface for
> Plain(Datagram)SocketImpl. I incorporated most of Marks suggestions.
> Also I introduced an optimization for
> VMPlainDatagramSocketImpl.receive() which turns out to be significantly
> faster than before because it avoids copying stuff back and forth from
> java to native and vice versa.
>
> Am Mittwoch, den 07.12.2005, 13:35 +0100 schrieb Mark Wielaard:
>
> > On Fri, 2005-11-11 at 17:53 +0100, Mark Wielaard wrote:
> >
> > > On Wed, 2005-11-09 at 21:16 +0000, Roman Kennke wrote:
> > >
> > > > Ingo has abstracted out the native code from gnu.java.net.PlainSocketImpl
> > > > and gnu.java.net.PlainDatagramSocketImpl into a new VM interface. This
> > > > allows VM implementors to provide a different implementation for the
> > > > native parts of these classes if they wish. Is this ok to commit as it
> > > > is? Do you have any suggestions/improvements to the interface? We would
> > > > like to have a stable VM interface for this area, so maybe it would be
> > > > helpful to discuss this with other VM implementors...
> > >
> > > A first comment is that for the VM reference implementation I would just
> > > make all methods native directly, no need to chain them to some helper
> > > method here. Is there a reason that only nativeLeave() is synchronized?
>
>
> I changed that.
>
>
> > > After we agree on this VM interface (and I think it is good, I just want
> > > to try it out first) we can discuss the rest of the patch. Just one
> > > quick nitpick now already: please keep the boilerplate at the top of the
> > > file intact, some of the files in the patch have the old FSF address.
>
>
> Fixed.
>
>
> > - Do we need the boolean stream argument in
> >  VMPlainSocketImpl.create()?
>
>
> No we don't. I have changed the VM interface.
>
>
> > - VMPlainSocketImpl.bind() has as comment
> >  **** How bind to INADDR_ANY ****
> >  Should we make a convention for that one? For example just use null
> >  add addr?
>
>
> The API docs for Socket.bind() seem to suggest that null means
> INADDR_ANY. So I would do it like this too.
>
>
> > - There is a PlainSocketImpl.connect() method that takes a timeout
> >  argument. We provide a default implementation now, but shouldn't this
> >  version move to the VMPlainSocketImpl? I don't know if our default
> >  implementation is actually that create, maybe you want to do the
> >  actual timeout in a more specific way.
>
>
> I pulled that into the VM interface.
>
>
> > - Should we provide wrappers/unwrappers for the Integer and Boolean
> >  arguments/return values of setOption() and getOption() that call the
> >  VM versions with primitive types? That would make the JNI code much
> >  cleaner.
>
>
> I haven't done this yet because this would mean to change the native
> code significantly.
>
>
> > - PlainSocketImpl.sendUrgentData() should delegate to VMPlainSocketImpl.
>
>
> Pulled into the VM interface.
>
>
> > - It might makes sense to have a no argument read() and write() method
> >  in the VM interface instead of just the byte[] versions.
>
>
> Pulled in, however still wraps things up into byte[] to avoid heavy
> adjustments in native code at this point.
>
>
> > - We should explicitly document the new constant IP_TTL we use in
> >  PlainDatagramSocketImpl for the VM interface since it isn't defined as
> >  SocketOption. Or should we just make a special VM interface to set/get
> >  the TTL?
>
>
> I have pulled the constant into the VM interface.
>
>
> > - The PlainDatagramSocket multicast opertations join(Group),
> >  leave(Group) etc, need to go through to the VM interface.
>
>
> Pulled in.
>
> Is this ok to check in?
>
> 2005-12-16  Roman Kennke  <address@hidden>
>
>         * vm/reference/gnu/java/net/VMPlainSocketImpl.java: New VM
> class.
>         * vm/reference/gnu/java/net/VMPlainDatagramSocketImpl.java:
>         New VM class.
>         * native/jni/java-net/gnu_java_net_VMPlainSocketImpl.c: New
> file.
>         * native/jni/java-net/gnu_java_net_VMPlainDatagramSocketImpl.c:
>         New file.
>         * native/jni/java-net/gnu_java_net_PlainDatagramSocketImpl.c:
>         Removed.
>         * native/jni/java-net/gnu_java_net_PlainSocketImpl.c: Removed.
>         * native/jni/java-net/Makefile.am: Adjusted for new source
> files.
>         * gnu/java/net/PlainDatagramSocketImpl.java: Use new VM
> interface.
>         * gnu/java/net/PlainSocketImpl.java: Use new VM interface.
>         * include/gnu_java_net_PlainDatagramSocketImpl.h: Removed.
>         * include/gnu_java_net_PlainSocketImpl.h: Removed.
>         * include/gnu_java_net_VMPlainDatagramSocketImpl.h: New header
>         file.
>         * include/gnu_java_net_VMPlainSocketImpl.h: New header file.
>
>
> /Roman
>
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Classpath-patches mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/classpath-patches