Skip to content

Commit

Permalink
refactor: invert dependency between Message and Connection
Browse files Browse the repository at this point in the history
  • Loading branch information
@sangelovic
sangelovic committed Oct 1, 2024
1 parent 1b7acaa commit 5ae3916
Show file tree
Hide file tree
Showing 15 changed files with 233 additions and 177 deletions.
4 changes: 2 additions & 2 deletions include/sdbus-c++/IObject.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ namespace sdbus {
*
* @throws sdbus::Error in case of failure
*/
[[nodiscard]] virtual Signal createSignal(const InterfaceName& interfaceName, const SignalName& signalName) = 0;
[[nodiscard]] virtual Signal createSignal(const InterfaceName& interfaceName, const SignalName& signalName) const = 0;

/*!
* @brief Emits signal for this object path
Expand All @@ -419,7 +419,7 @@ namespace sdbus {
protected: // Internal API for efficiency reasons used by high-level API helper classes
friend SignalEmitter;

[[nodiscard]] virtual Signal createSignal(const char* interfaceName, const char* signalName) = 0;
[[nodiscard]] virtual Signal createSignal(const char* interfaceName, const char* signalName) const = 0;
};

// Out-of-line member definitions
Expand Down
4 changes: 2 additions & 2 deletions include/sdbus-c++/IProxy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ namespace sdbus {
*
* @throws sdbus::Error in case of failure
*/
[[nodiscard]] virtual MethodCall createMethodCall(const InterfaceName& interfaceName, const MethodName& methodName) = 0;
[[nodiscard]] virtual MethodCall createMethodCall(const InterfaceName& interfaceName, const MethodName& methodName) const = 0;

/*!
* @brief Calls method on the remote D-Bus object
Expand Down Expand Up @@ -652,7 +652,7 @@ namespace sdbus {
friend AsyncMethodInvoker;
friend SignalSubscriber;

[[nodiscard]] virtual MethodCall createMethodCall(const char* interfaceName, const char* methodName) = 0;
[[nodiscard]] virtual MethodCall createMethodCall(const char* interfaceName, const char* methodName) const = 0;
virtual void registerSignalHandler( const char* interfaceName
, const char* signalName
, signal_handler signalHandler ) = 0;
Expand Down
12 changes: 6 additions & 6 deletions include/sdbus-c++/Message.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ namespace sdbus {
class UnixFd;
class MethodReply;
namespace internal {
class ISdBus;
class IConnection;
}
}

Expand Down Expand Up @@ -245,15 +245,15 @@ namespace sdbus {

protected:
Message() = default;
explicit Message(internal::ISdBus* sdbus) noexcept;
Message(void *msg, internal::ISdBus* sdbus) noexcept;
Message(void *msg, internal::ISdBus* sdbus, adopt_message_t) noexcept;
explicit Message(internal::IConnection* connection) noexcept;
Message(void *msg, internal::IConnection* connection) noexcept;
Message(void *msg, internal::IConnection* connection, adopt_message_t) noexcept;

friend Factory;

protected:
void* msg_{};
internal::ISdBus* sdbus_{};
internal::IConnection* connection_{};
mutable bool ok_{true};
};

Expand All @@ -275,7 +275,7 @@ namespace sdbus {
bool doesntExpectReply() const;

protected:
MethodCall(void *msg, internal::ISdBus* sdbus, adopt_message_t) noexcept;
MethodCall(void *msg, internal::IConnection* connection, adopt_message_t) noexcept;

private:
MethodReply sendWithReply(uint64_t timeout = 0) const;
Expand Down
143 changes: 100 additions & 43 deletions src/Connection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,6 @@ Connection::PollData Connection::getEventLoopPollData() const
return {pollData.fd, pollData.events, timeout, eventFd_.fd};
}

const ISdBus& Connection::getSdBusInterface() const
{
return *sdbus_.get();
}

ISdBus& Connection::getSdBusInterface()
{
return *sdbus_.get();
}

void Connection::addObjectManager(const ObjectPath& objectPath)
{
auto r = sdbus_->sd_bus_add_object_manager(bus_.get(), nullptr, objectPath.c_str());
Expand Down Expand Up @@ -488,7 +478,7 @@ PlainMessage Connection::createPlainMessage() const

SDBUS_THROW_ERROR_IF(r < 0, "Failed to create a plain message", -r);

return Message::Factory::create<PlainMessage>(sdbusMsg, sdbus_.get(), adopt_message);
return Message::Factory::create<PlainMessage>(sdbusMsg, const_cast<Connection*>(this), adopt_message);
}

MethodCall Connection::createMethodCall( const ServiceName& destination
Expand All @@ -515,7 +505,7 @@ MethodCall Connection::createMethodCall( const char* destination

SDBUS_THROW_ERROR_IF(r < 0, "Failed to create method call", -r);

return Message::Factory::create<MethodCall>(sdbusMsg, sdbus_.get(), adopt_message);
return Message::Factory::create<MethodCall>(sdbusMsg, const_cast<Connection*>(this), adopt_message);
}

Signal Connection::createSignal( const ObjectPath& objectPath
Expand All @@ -535,34 +525,7 @@ Signal Connection::createSignal( const char* objectPath

SDBUS_THROW_ERROR_IF(r < 0, "Failed to create signal", -r);

return Message::Factory::create<Signal>(sdbusMsg, sdbus_.get(), adopt_message);
}

MethodReply Connection::callMethod(const MethodCall& message, uint64_t timeout)
{
// If the call expects reply, this call will block the bus connection from
// serving other messages until the reply arrives or the call times out.
auto reply = message.send(timeout);

// Wake up event loop to process messages that may have arrived in the meantime...
wakeUpEventLoopIfMessagesInQueue();

return reply;
}

Slot Connection::callMethod(const MethodCall& message, void* callback, void* userData, uint64_t timeout, return_slot_t)
{
// TODO: Think of ways of optimizing these three locking/unlocking of sdbus mutex (merge into one call?)
auto timeoutBefore = getEventLoopPollData().timeout;
auto slot = message.send(callback, userData, timeout, return_slot);
auto timeoutAfter = getEventLoopPollData().timeout;

// An event loop may wait in poll with timeout `t1', while in another thread an async call is made with
// timeout `t2'. If `t2' < `t1', then we have to wake up the event loop thread to update its poll timeout.
if (timeoutAfter < timeoutBefore)
notifyEventLoopToWakeUpFromPoll();

return slot;
return Message::Factory::create<Signal>(sdbusMsg, const_cast<Connection*>(this), adopt_message);
}

void Connection::emitPropertiesChangedSignal( const ObjectPath& objectPath
Expand Down Expand Up @@ -648,6 +611,100 @@ Slot Connection::registerSignalHandler( const char* sender
return {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref((sd_bus_slot*)slot); }};
}

sd_bus_message* Connection::incrementMessageRefCount(sd_bus_message* sdbusMsg)
{
return sdbus_->sd_bus_message_ref(sdbusMsg);
}

sd_bus_message* Connection::decrementMessageRefCount(sd_bus_message* sdbusMsg)
{
return sdbus_->sd_bus_message_unref(sdbusMsg);
}

int Connection::querySenderCredentials(sd_bus_message* sdbusMsg, uint64_t mask, sd_bus_creds **creds)
{
return sdbus_->sd_bus_query_sender_creds(sdbusMsg, mask, creds);
}

sd_bus_creds* Connection::incrementCredsRefCount(sd_bus_creds* creds)
{
return sdbus_->sd_bus_creds_ref(creds);
}

sd_bus_creds* Connection::decrementCredsRefCount(sd_bus_creds* creds)
{
return sdbus_->sd_bus_creds_unref(creds);
}

sd_bus_message* Connection::callMethod(sd_bus_message* sdbusMsg, uint64_t timeout)
{
sd_bus_error sdbusError = SD_BUS_ERROR_NULL;
SCOPE_EXIT{ sd_bus_error_free(&sdbusError); };

// This call will block the bus connection from serving other messages
// until the reply arrives or the call times out.
sd_bus_message* sdbusReply{};
auto r = sdbus_->sd_bus_call(nullptr, sdbusMsg, timeout, &sdbusError, &sdbusReply);

if (sd_bus_error_is_set(&sdbusError))
throw Error(Error::Name{sdbusError.name}, sdbusError.message);

SDBUS_THROW_ERROR_IF(r < 0, "Failed to call method", -r);

// Wake up event loop to process messages that may have arrived in the meantime...
wakeUpEventLoopIfMessagesInQueue();

return sdbusReply;
}

Slot Connection::callMethodAsync(sd_bus_message* sdbusMsg, sd_bus_message_handler_t callback, void* userData, uint64_t timeout, return_slot_t)
{
sd_bus_slot *slot{};

// TODO: Think of ways of optimizing these three locking/unlocking of sdbus mutex (merge into one call?)
auto timeoutBefore = getEventLoopPollData().timeout;
auto r = sdbus_->sd_bus_call_async(nullptr, &slot, sdbusMsg, (sd_bus_message_handler_t)callback, userData, timeout);
SDBUS_THROW_ERROR_IF(r < 0, "Failed to call method asynchronously", -r);
auto timeoutAfter = getEventLoopPollData().timeout;

// An event loop may wait in poll with timeout `t1', while in another thread an async call is made with
// timeout `t2'. If `t2' < `t1', then we have to wake up the event loop thread to update its poll timeout.
if (timeoutAfter < timeoutBefore)
notifyEventLoopToWakeUpFromPoll();

return {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref((sd_bus_slot*)slot); }};
}

void Connection::sendMessage(sd_bus_message* sdbusMsg)
{
auto r = sdbus_->sd_bus_send(nullptr, sdbusMsg, nullptr);

SDBUS_THROW_ERROR_IF(r < 0, "Failed to send D-Bus message", -r);
}

sd_bus_message* Connection::createMethodReply(sd_bus_message* sdbusMsg)
{
sd_bus_message* sdbusReply{};

auto r = sdbus_->sd_bus_message_new_method_return(sdbusMsg, &sdbusReply);
SDBUS_THROW_ERROR_IF(r < 0, "Failed to create method reply", -r);

return sdbusReply;
}

sd_bus_message* Connection::createErrorReplyMessage(sd_bus_message* sdbusMsg, const Error& error)
{
sd_bus_error sdbusError = SD_BUS_ERROR_NULL;
SCOPE_EXIT{ sd_bus_error_free(&sdbusError); };
sd_bus_error_set(&sdbusError, error.getName().c_str(), error.getMessage().c_str());

sd_bus_message* sdbusErrorReply{};
auto r = sdbus_->sd_bus_message_new_method_error(sdbusMsg, &sdbusErrorReply, &sdbusError);
SDBUS_THROW_ERROR_IF(r < 0, "Failed to create method error reply", -r);

return sdbusErrorReply;
}

Connection::BusPtr Connection::openBus(const BusFactory& busFactory)
{
sd_bus* bus{};
Expand Down Expand Up @@ -785,7 +842,7 @@ Message Connection::getCurrentlyProcessedMessage() const
{
auto* sdbusMsg = sdbus_->sd_bus_get_current_message(bus_.get());

return Message::Factory::create<Message>(sdbusMsg, sdbus_.get());
return Message::Factory::create<Message>(sdbusMsg, const_cast<Connection*>(this));
}

template <typename StringBasedType>
Expand All @@ -804,7 +861,7 @@ int Connection::sdbus_match_callback(sd_bus_message *sdbusMessage, void *userDat
assert(matchInfo != nullptr);
assert(matchInfo->callback);

auto message = Message::Factory::create<PlainMessage>(sdbusMessage, &matchInfo->connection.getSdBusInterface());
auto message = Message::Factory::create<PlainMessage>(sdbusMessage, &matchInfo->connection);

auto ok = invokeHandlerAndCatchErrors([&](){ matchInfo->callback(std::move(message)); }, retError);

Expand All @@ -817,7 +874,7 @@ int Connection::sdbus_match_install_callback(sd_bus_message *sdbusMessage, void
assert(matchInfo != nullptr);
assert(matchInfo->installCallback);

auto message = Message::Factory::create<PlainMessage>(sdbusMessage, &matchInfo->connection.getSdBusInterface());
auto message = Message::Factory::create<PlainMessage>(sdbusMessage, &matchInfo->connection);

auto ok = invokeHandlerAndCatchErrors([&](){ matchInfo->installCallback(std::move(message)); }, retError);

Expand Down
20 changes: 14 additions & 6 deletions src/Connection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,6 @@ namespace sdbus::internal {
void detachSdEventLoop() override;
sd_event *getSdEventLoop() override;

[[nodiscard]] const ISdBus& getSdBusInterface() const override;
[[nodiscard]] ISdBus& getSdBusInterface() override;

Slot addObjectVTable( const ObjectPath& objectPath
, const InterfaceName& interfaceName
, const sd_bus_vtable* vtable
Expand All @@ -145,9 +142,6 @@ namespace sdbus::internal {
, const char* interfaceName
, const char* signalName ) const override;

MethodReply callMethod(const MethodCall& message, uint64_t timeout) override;
Slot callMethod(const MethodCall& message, void* callback, void* userData, uint64_t timeout, return_slot_t) override;

void emitPropertiesChangedSignal( const ObjectPath& objectPath
, const InterfaceName& interfaceName
, const std::vector<PropertyName>& propNames ) override;
Expand All @@ -169,6 +163,20 @@ namespace sdbus::internal {
, void* userData
, return_slot_t ) override;

sd_bus_message* incrementMessageRefCount(sd_bus_message* sdbusMsg) override;
sd_bus_message* decrementMessageRefCount(sd_bus_message* sdbusMsg) override;

int querySenderCredentials(sd_bus_message* sdbusMsg, uint64_t mask, sd_bus_creds **creds) override;
sd_bus_creds* incrementCredsRefCount(sd_bus_creds* creds) override;
sd_bus_creds* decrementCredsRefCount(sd_bus_creds* creds) override;

sd_bus_message* callMethod(sd_bus_message* sdbusMsg, uint64_t timeout) override;
Slot callMethodAsync(sd_bus_message* sdbusMsg, sd_bus_message_handler_t callback, void* userData, uint64_t timeout, return_slot_t) override;
void sendMessage(sd_bus_message* sdbusMsg) override;

sd_bus_message* createMethodReply(sd_bus_message* sdbusMsg) override;
sd_bus_message* createErrorReplyMessage(sd_bus_message* sdbusMsg, const Error& error) override;

private:
using BusFactory = std::function<int(sd_bus**)>;
using BusPtr = std::unique_ptr<sd_bus, std::function<sd_bus*(sd_bus*)>>;
Expand Down
30 changes: 20 additions & 10 deletions src/IConnection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ namespace sdbus {
using MethodName = MemberName;
using SignalName = MemberName;
using PropertyName = MemberName;
class Error;
namespace internal {
class ISdBus;
}
Expand All @@ -64,9 +65,6 @@ namespace sdbus::internal {
public:
~IConnection() override = default;

[[nodiscard]] virtual const ISdBus& getSdBusInterface() const = 0;
[[nodiscard]] virtual ISdBus& getSdBusInterface() = 0;

[[nodiscard]] virtual Slot addObjectVTable( const ObjectPath& objectPath
, const InterfaceName& interfaceName
, const sd_bus_vtable* vtable
Expand All @@ -89,13 +87,6 @@ namespace sdbus::internal {
, const char* interfaceName
, const char* signalName ) const = 0;

virtual MethodReply callMethod(const MethodCall& message, uint64_t timeout) = 0;
[[nodiscard]] virtual Slot callMethod( const MethodCall& message
, void* callback
, void* userData
, uint64_t timeout
, return_slot_t ) = 0;

virtual void emitPropertiesChangedSignal( const ObjectPath& objectPath
, const InterfaceName& interfaceName
, const std::vector<PropertyName>& propNames ) = 0;
Expand All @@ -116,6 +107,25 @@ namespace sdbus::internal {
, sd_bus_message_handler_t callback
, void* userData
, return_slot_t ) = 0;

virtual sd_bus_message* incrementMessageRefCount(sd_bus_message* sdbusMsg) = 0;
virtual sd_bus_message* decrementMessageRefCount(sd_bus_message* sdbusMsg) = 0;

// TODO: Refactor to higher level (Creds class will ownership handling and getters)
virtual int querySenderCredentials(sd_bus_message* sdbusMsg, uint64_t mask, sd_bus_creds **creds) = 0;
virtual sd_bus_creds* incrementCredsRefCount(sd_bus_creds* creds) = 0;
virtual sd_bus_creds* decrementCredsRefCount(sd_bus_creds* creds) = 0;

virtual sd_bus_message* callMethod(sd_bus_message* sdbusMsg, uint64_t timeout) = 0;
[[nodiscard]] virtual Slot callMethodAsync( sd_bus_message* sdbusMsg
, sd_bus_message_handler_t callback
, void* userData
, uint64_t timeout
, return_slot_t ) = 0;
virtual void sendMessage(sd_bus_message* sdbusMsg) = 0;

virtual sd_bus_message* createMethodReply(sd_bus_message* sdbusMsg) = 0;
virtual sd_bus_message* createErrorReplyMessage(sd_bus_message* sdbusMsg, const Error& error) = 0;
};

[[nodiscard]] std::unique_ptr<sdbus::internal::IConnection> createPseudoConnection();
Expand Down
1 change: 1 addition & 0 deletions src/ISdBus.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ namespace sdbus::internal {
virtual int sd_bus_message_set_destination(sd_bus_message *m, const char *destination) = 0;

virtual int sd_bus_query_sender_creds(sd_bus_message *m, uint64_t mask, sd_bus_creds **c) = 0;
virtual sd_bus_creds* sd_bus_creds_ref(sd_bus_creds *c) = 0;
virtual sd_bus_creds* sd_bus_creds_unref(sd_bus_creds *c) = 0;

virtual int sd_bus_creds_get_pid(sd_bus_creds *c, pid_t *pid) = 0;
Expand Down
Loading

0 comments on commit 5ae3916

Please sign in to comment.