When to close a passed file descriptor? about sdbus-cpp HOT 7 CLOSED

If I understand sdbus correctly it should be safe to close it after Message& Message::operator<<(const UnixFd &item) (or sd_bus_message_append_basic)

https://www.freedesktop.org/software/systemd/man/sd_bus_message_append_basic.html

If type is "h" (UNIX file descriptor), the descriptor is duplicated by this call and the passed descriptor stays in possession of the caller.

So it should be possible with the low-level API, but not the convenience API. I would still like to use the convenience API and generated stubs for the other methods and properties. Is there a way to mix all that?

from sdbus-cpp.

Hi, UnixFd was meant as a simple, thin wrapper for fd to allow sdbus-c++ type system to distinguish it from the general int type, without any additional logic.

But you're right, it is impossible to close a fd after putting it into a method reply message on server side using convenience API, because the fd is put into the message after the method handler body.

And yes, a way to solve this would be to use RAII, i.e. close the fd in UnixFd's destructor -- but only if the client wishes so. An additional boolean member of UnixFd and simple close() in the d-tor could do. For higher flexibility (maybe simple close() with ignoring the return value is too limiting?), we could even add a deleter in unique_ptr's style, but UnixFd would have to change to a class template (version with no closing the fd would be UnixFd<>).

What do you think?

from sdbus-cpp.

I was thinking about trying to add an owning UniqueUnixFd type, and creating an alias like using AnyUnixFd = std::variant<UnixFd, UniqueUnixFd>. The point of the variant is for using the same type in generated stubs, so there is no need to annotate the xml with the wanted type. I think using a template UnixFd<> won't work because of this.

Returning an integer would implicitly use the first type, using the owning fd would require calling its explicit constructor. Implementing operator<< is a simple call to std::visit. I don't think operator>> make sense for the variant, so one of the type must be chosen for receiving file descriptors (I would have a preference for the owning version for this case, since you are owning the descriptor you've just received, but both would work).

from sdbus-cpp.

Hi guys,

again an interesting topic.

What came to my mind was to make the method async on the server side but directly calling 'returnResults' to return the FD and closing the local one directly afterwards before the incoming method call ends.
Might this be a work around for now?

I also came across another issue with the FDs on the receiver side.
It seems that in Message& Message::operator>>(UnixFd &item) we also have to dup the FD because the original received FD is owned by and closed together with the low level SD-BUS C message object.

Think for that an UnixFD class that handles ownership and closes the FD if needed might also be really handy.

For testing I adapted the code to the following:

Message& Message::operator>>(UnixFd &item)e
{
    int fd = -1;
    auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UNIX_FD, &fd);
    if (r == 0)
        ok_ = false;

    SDBUS_THROW_ERROR_IF(r < 0, "Failed to deserialize a UnixFd value", -r);

    item.fd_ = ::dup(fd);

    return *this;
}

from sdbus-cpp.

I think it is best to let the user duplicate the file descriptor as needed.

I thought the received fd was owned, but I did not test anything yet. There is no mention of the file descriptor lifetime in sd_bus_message_read_basic doc, only that the strings are borrowed.

from sdbus-cpp.

I've just checked libsystemd code, file descriptors are indeed closed when the message is freed: https://github.com/systemd/systemd/blob/a4eb991831099756e9debc2cfeaf689584deed08/src/libsystemd/sd-bus/bus-message.c#L125

from sdbus-cpp.

There is no mention of the file descriptor lifetime in sd_bus_message_read_basic doc, only that the strings are borrowed.

I also couldn't find anything mentioned in the docu but in the source it is quite clear the the FDs are closed when the sd_bus_message is freed:
In static sd_bus_message* message_free(sd_bus_message *m) they do:

// File: libsystemd/sd-bus/bus-message.c
// Line: 124
if (m->free_fds) {
    close_many(m->fds, m->n_fds);
    free(m->fds);
}

from sdbus-cpp.