[prev in list] [next in list] [prev in thread] [next in thread]
List: freedesktop-dbus
Subject: Re: RFC: adding fd-passing to win32
From: Marc-André Lureau <marcandre.lureau () gmail ! com>
Date: 2022-08-09 7:51:38
Message-ID: CAJ+F1CKs_8ebDB=dQLbpVwN6gNMnntu_SppcOFjeGj+QrrRWuw () mail ! gmail ! com
[Download RAW message or body]
Hi Thiago
On Mon, Aug 8, 2022 at 8:19 PM Thiago Macieira <thiago@kde.org> wrote:
> On Monday, 8 August 2022 04:40:22 PDT Marc-André Lureau wrote:
> > FDs are not HANDLEs, we shouldn't mix the two (a mistake commonly
> observed
> > and annoying to clean up).
>
> I never claimed otherwise. My point is that the wire format is an
> implementation detail.
>
> > If some day Windows sockets learn SCM_RIGHT, we should transfer FDs
> values.
>
> Highly doubtful. It's doubtful that it'll learn that Linux-specific
> feature and
> it's doubtful that file descriptor values will be transferable. They
> aren't on
> Linux anyway: the file descriptor value is not retained. You get a new
> file
> descriptor locally, which happens to map to the same kernel-side structure.
>
>
If we are able to find solutions at D-Bus level, it's not impossible they
implement better low-level solutions in afunix.sys. See also cygwin
discussion pointed by Lawrence.
> > We could eventually use 'h' for FDs or HANDLEs or SOCKETs (or other kind
> as
> > necessary), the "handle array" contains the type details. But then 'h'
> > implementation will be different on Windows, regardless of potentially
> > future same SCM_RIGHT support. I think it's a better idea to treat
> HANDLEs
> > as a different type.
>
> I disagree.
>
> Let's make this easier for you:
>
> If you use 'h' and just make the reference implementation work, then all
> the
> implementations that use it will automatically gain the functionality.
> You'll
> only need to do code reviews, without a spec change, with most of the
> changes
> restricted to win32-specific files. You'll need to update the
> implementations
> that do D-Bus socket directly, but similarly they will not have any user
> API
> change and applications should work unmodified.
>
My point is that 'h' is the right type to transfer FDs, not HANDLEs/SOCKET
etc.
>
> If you use 'H', you'll first need to start with a spec update and you'll
> need
> to convince people like Simon and myself, who don't have as extensive
> Windows
> knowledge, that it is needed (and this thread is pointing that we aren't
> getting convinced). You'll need code reviews throughout the implementation
> to
>
Spec update will be needed for nego phase, details of HANDLE transfer etc.
Reference implementation changes are required anyway.
Frankly, adding 'H' is minor compared to the rest of the changes involved.
> support a new type. Then you'll need to update the API in libdbus-1, then
> all
> the implementations that use it, including fixing them where they expect
> to
> know the full population of D-Bus types but now don't. That is a much
> tougher
> ask.
>
Well, I have working PoCs for gdbus & zbus already. Only libdbus is a bit
more difficult to deal with.
Regarding compatibility, the 'H' type won't be used unless capability is
negotiated.
>
> Maybe your objective has never been to retain source compatibility across
> platforms. But then please explain why that is not a goal.
>
It would be great if we could reuse our existing interfaces using 'h', with
SCM_RIGHT or not. Unfortunately, that's not the case. You need
HANDLEs/SOCKETs to be able to call DuplicateHandle or WSADuplicateSocket or
close appropriately. We can try to make guesses on the underlying objects
(by mapping FD->HANDLE->SOCKET for ex), but that would be a bit fragile and
not necessarily the most convenient API to use for the client on win32:
SOCKET s = WSASocket(..)
int fd = _open_osfhandle(s,..)
dbus_message_append_args(.., DBUS_TYPE_UNIX_FD, &fd,..)
In libdbus, or receiver
dbus_message_get_args(.., DBUS_TYPE_UNIX_FD, &fd)
HANDLE h = _get_osfhandle(fd)
if (is_socket(h)) /* see
https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/gwin32.c#L1457 for ex
*/
{
SOCKET s = (SOCKET)h;
..
WSADuplicateSocket(s, ) to target process for ex..
..
closesocket(s) /* in theory better than _close(), but this also unclear!
*/
} else {
DuplicateHandle(h, )..
_close(fd)
}
If we expose HANDLE & SOCKETs directly instead (both in APIs and protocol),
we don't need to juggle with the fd back/forth mapping and potential
guesses & pitfalls.
--
Marc-André Lureau
[Attachment #3 (text/html)]
<div dir="ltr"><div dir="ltr">Hi Thiago<br></div><br><div class="gmail_quote"><div \
dir="ltr" class="gmail_attr">On Mon, Aug 8, 2022 at 8:19 PM Thiago Macieira <<a \
href="mailto:thiago@kde.org" target="_blank">thiago@kde.org</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Monday, 8 August \
2022 04:40:22 PDT Marc-André Lureau wrote:<br> > FDs are not HANDLEs, we \
shouldn't mix the two (a mistake commonly observed<br> > and annoying to clean \
up).<br> <br>
I never claimed otherwise. My point is that the wire format is an <br>
implementation detail.<br>
<br>
> If some day Windows sockets learn SCM_RIGHT, we should transfer FDs values.<br>
<br>
Highly doubtful. It's doubtful that it'll learn that Linux-specific feature \
and <br> it's doubtful that file descriptor values will be transferable. They \
aren't on <br> Linux anyway: the file descriptor value is not retained. You get a \
new file <br> descriptor locally, which happens to map to the same kernel-side \
structure.<br> <br></blockquote><div><br></div><div>If we are able to find solutions \
at D-Bus level, it's not impossible they implement better low-level solutions in \
afunix.sys. See also cygwin discussion pointed by Lawrence.<br></div><div> \
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px \
solid rgb(204,204,204);padding-left:1ex"> > We could eventually use 'h' \
for FDs or HANDLEs or SOCKETs (or other kind as<br> > necessary), the "handle \
array" contains the type details. But then 'h'<br> > implementation \
will be different on Windows, regardless of potentially<br> > future same \
SCM_RIGHT support. I think it's a better idea to treat HANDLEs<br> > as a \
different type.<br> <br>
I disagree.<br>
<br>
Let's make this easier for you:<br>
<br>
If you use 'h' and just make the reference implementation work, then all the \
<br> implementations that use it will automatically gain the functionality. \
You'll <br> only need to do code reviews, without a spec change, with most of the \
changes <br> restricted to win32-specific files. You'll need to update the \
implementations <br> that do D-Bus socket directly, but similarly they will not have \
any user API <br> change and applications should work \
unmodified.<br></blockquote><div><br></div><div>My point is that 'h' is the \
right type to transfer FDs, not HANDLEs/SOCKET etc.<br></div><div> </div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"> <br>
If you use 'H', you'll first need to start with a spec update and \
you'll need <br> to convince people like Simon and myself, who don't have as \
extensive Windows <br> knowledge, that it is needed (and this thread is pointing that \
we aren't <br> getting convinced). You'll need code reviews throughout the \
implementation to <br></blockquote><div><br></div><div>Spec update will be needed for \
nego phase, details of HANDLE transfer etc. Reference implementation changes are \
required anyway.</div><div><br></div><div>Frankly, adding 'H' is minor \
compared to the rest of the changes involved.<br></div><div> <br></div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"> support a new type. Then you'll need to \
update the API in libdbus-1, then all <br> the implementations that use it, including \
fixing them where they expect to <br> know the full population of D-Bus types but now \
don't. That is a much tougher <br> ask.<br></blockquote><div><br></div><div>Well, \
I have working PoCs for gdbus & zbus already. Only libdbus is a bit more \
difficult to deal with.<br></div><div><br></div><div>Regarding compatibility, the \
'H' type won't be used unless capability is negotiated.<br></div><div> \
<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br>
Maybe your objective has never been to retain source compatibility across <br>
platforms. But then please explain why that is not a goal.<br>
</blockquote></div><div><br></div><div>It would be great if we could reuse our \
existing interfaces using 'h', with SCM_RIGHT or not. Unfortunately, \
that's not the case. You need HANDLEs/SOCKETs to be able to call DuplicateHandle \
or WSADuplicateSocket or close appropriately. We can try to make guesses on the \
underlying objects (by mapping FD->HANDLE->SOCKET for ex), but that would be a \
bit fragile and not necessarily the most convenient API to use for the client on \
win32:</div><div><br></div><div>SOCKET s = WSASocket(..)</div><div>int fd = \
_open_osfhandle(s,..)<br></div><div>dbus_message_append_args(.., DBUS_TYPE_UNIX_FD, \
&fd,..)</div><div><br></div><div>In libdbus, or \
receiver<br></div><div>dbus_message_get_args(.., DBUS_TYPE_UNIX_FD, \
&fd)</div><div>HANDLE h = _get_osfhandle(fd)</div><div>if (is_socket(h)) /* see \
<a href="https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/gwin32.c#L1457" \
target="_blank">https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/gwin32.c#L1457</a> \
for ex */</div><div>{<br></div><div> SOCKET s = (SOCKET)h;</div><div> \
..</div><div> WSADuplicateSocket(s, ) to target process for ex..</div><div> \
..<br></div><div> closesocket(s) /* in theory better than _close(), but this also \
unclear! */<br></div><div>} else {</div><div> DuplicateHandle(h, )..<br></div><div> \
_close(fd)</div><div>}<br></div><div><br></div><div>If we expose HANDLE & SOCKETs \
directly instead (both in APIs and protocol), we don't need to juggle with the fd \
back/forth mapping and potential guesses & pitfalls.<br></div><div><br></div>-- \
<br><div dir="ltr">Marc-André Lureau<br></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic