Re: Review of gnio, round 1

[Dan Winship <danw gnome org>]

(I still haven't read all of gnio, so I'm mostly only commenting on
Alex's comments, plus GSocket, since I have read that.)

Alexander Larsson wrote:
> Every time I've done socket code I've enabled SO_REUSEADDR

Yeah, Ryan and I had this exact conversation (including the same link)
on IRC yesterday.

There are also other flags we might want to think about. In libsoup, all
sockets get marked FD_CLOEXEC because of a long-ago bug. (The Red Carpet
daemon used librpm to do stuff, and librpm would spawn postinstall
scripts without doing the necessary cleanup like closing fds, and so
sometimes you'd try to restart rcd, and it would fail to start up again,
because some process like cupsd which had previously been restarted by
rcd now was holding a leaked copy of the old rcd process's listening
socket, and so the new rcd process couldn't re-bind that address.)
Anyway, ideally people would use spawning APIs that cleaned up fds
properly, but the fact is, unless you're writing inetd, you generally
don't intend to hand off your sockets to child processes anyway, so
setting FD_CLOEXEC unconditionally (or using SOCK_CLOEXEC where
available) generally makes sense.

> Win32 issues:

I'd filed a bug about some other win32 issues over the weekend.
(http://bugzilla.gnome.org/show_bug.cgi?id=580236)

> * Need to check errors from WSAStartup

Would it be better to have only one place in gio that calls WSAStartup?
(Currently that's g_inet_address_get_type().)

> We don't currently allow passing in the protocol to the socket. This
> means we can't for instance use SCTP (the upcomming TCP replacement)
> with GSocket.

I imagine the theory is that G_SOCKET_TYPE_STREAM,
G_SOCKET_TYPE_DATAGRAM, and G_SOCKET_TYPE_SEQPACKET invariably also
imply tcp, udp, and sctp, respectively.

> g_socket_finalize closes socket even if g_socket_close() is already
> closed, need to add an is_closed variable. We should also check this in
> all operations so that we don't accidentally call stuff on an fd that
> may have been reused by now.

libsoup's solution here is to implement "close" as
shutdown(fd,SHUT_RDWR), and only actually call close(fd) at finalization
time.

> There is some weird g_warnings about "ABI compatibility" that don't
> really make sense to show the users.

Yeah, we should ensure that at compile time and remove the checks.

> GSocketConnection *g_socket_connection_connect_to_host (const gchar
> *host,
>                                                         const gchar
> *default_port,
> 							GCancellable        *cancellable,
>   							GError             **error);

IMHO, ports should always be numbers, not strings. getservbyname() is
useless, because you want your app to work on last year's distro that
doesn't know about this year's protocols. So apps should just provide a
#define for their port number and use that, rather than hoping that the
service's name will be in /etc/services.

> The unix socket code really should support linux-style abstract
> socketnames. Ideally in some way that makes it easy to fall back if this
> is not supported.

My vision of how this works is something like:

    - GUnixSocket{Connection,Service} - does unix sockets. Has APIs for
      using abstract names, but they return G_IO_ERROR_NOT_SUPPORTED on
      non-Linux.

    - GWin32NamedPipe{Connection,Service} - a Windows named pipe.
      Implements GIOStream, but is not related to any of the socket
      types.

    - GNamedBidirectionalIPC{Connection,Service} - Takes an
      application name and a channel name, and a flag for "session" or
      "system" level, and then "it does something", such that if one
      app makes the server and another app makes a client with the
      same parameters, then they can talk to each other. On Windows,
      this would use a named pipe, on Linux it would use abstract
      sockets, on other unixes, it would use a unix socket in /tmp.
      Or maybe it passes fds over D-Bus. And maybe on OS X it uses
      Mach ports.

So then, if you want to talk to an app that uses unix sockets, you
create a GUnixSocketConnection (and you don't need to worry about
fallback, because obviously you're not going to be trying to connect to
a service running on an abstract socket on a platform that doesn't
support them). And likewise if you want to connect to a windows named
pipe, you create a GWin32NamedPipeConnection. But if your goal is just
"make it possible for other apps to communicate with my app efficiently
in a standardized way", then you use the abstract service class, and it
does the right thing for your platform, and the client apps use the
abstract connection class, and it knows how to connect.

-- Dan