feedc0de/sdbus-cpp
1
0
Fork 0
forked from Kistler-Group/sdbus-cpp
Code Pull Requests Activity

Fix potential race condition in Object destruction

Browse Source
This commit is contained in:
Stanislav Angelovic
2021-07-22 17:33:59 +02:00
committed by Stanislav Angelovič
parent 8320429ef7
commit d77bb6b869
2 changed files with 40 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
src/Object.cpp
View File

@ -73,7 +73,7 @@ void Object::registerMethod( const std::string& interfaceName
{ {
SDBUS_THROW_ERROR_IF(!methodCallback, "Invalid method callback provided", EINVAL); SDBUS_THROW_ERROR_IF(!methodCallback, "Invalid method callback provided", EINVAL);
auto& interface = interfaces_[interfaceName]; auto& interface = getInterface(interfaceName);
InterfaceData::MethodData methodData{ std::move(inputSignature) InterfaceData::MethodData methodData{ std::move(inputSignature)
, std::move(outputSignature) , std::move(outputSignature)
, paramNamesToString(inputNames) + paramNamesToString(outputNames) , paramNamesToString(inputNames) + paramNamesToString(outputNames)
@ -98,7 +98,7 @@ void Object::registerSignal( const std::string& interfaceName
, const std::vector<std::string>& paramNames , const std::vector<std::string>& paramNames
, Flags flags ) , Flags flags )
{ {
auto& interface = interfaces_[interfaceName]; auto& interface = getInterface(interfaceName);
InterfaceData::SignalData signalData{std::move(signature), paramNamesToString(paramNames), std::move(flags)}; InterfaceData::SignalData signalData{std::move(signature), paramNamesToString(paramNames), std::move(flags)};
auto inserted = interface.signals.emplace(std::move(signalName), std::move(signalData)).second; auto inserted = interface.signals.emplace(std::move(signalName), std::move(signalData)).second;
@ -129,12 +129,12 @@ void Object::registerProperty( const std::string& interfaceName
{ {
SDBUS_THROW_ERROR_IF(!getCallback && !setCallback, "Invalid property callbacks provided", EINVAL); SDBUS_THROW_ERROR_IF(!getCallback && !setCallback, "Invalid property callbacks provided", EINVAL);
auto& interface = interfaces_[interfaceName]; auto& interface = getInterface(interfaceName);
InterfaceData::PropertyData propertyData{ std::move(signature) InterfaceData::PropertyData propertyData{ std::move(signature)
, std::move(getCallback) , std::move(getCallback)
, std::move(setCallback) , std::move(setCallback)
, std::move(flags)}; , std::move(flags) };
auto inserted = interface.properties.emplace(std::move(propertyName), std::move(propertyData)).second; auto inserted = interface.properties.emplace(std::move(propertyName), std::move(propertyData)).second;
SDBUS_THROW_ERROR_IF(!inserted, "Failed to register property: property already exists", EINVAL); SDBUS_THROW_ERROR_IF(!inserted, "Failed to register property: property already exists", EINVAL);
@ -142,7 +142,7 @@ void Object::registerProperty( const std::string& interfaceName
void Object::setInterfaceFlags(const std::string& interfaceName, Flags flags) void Object::setInterfaceFlags(const std::string& interfaceName, Flags flags)
{ {
auto& interface = interfaces_[interfaceName]; auto& interface = getInterface(interfaceName);
interface.flags = flags; interface.flags = flags;
} }
@ -236,6 +236,11 @@ const Message* Object::getCurrentlyProcessedMessage() const
return m_CurrentlyProcessedMessage.load(std::memory_order_relaxed); return m_CurrentlyProcessedMessage.load(std::memory_order_relaxed);
} }
Object::InterfaceData& Object::getInterface(const std::string& interfaceName)
{
return interfaces_.emplace(interfaceName, *this).first->second;
}
const std::vector<sd_bus_vtable>& Object::createInterfaceVTable(InterfaceData& interfaceData) const std::vector<sd_bus_vtable>& Object::createInterfaceVTable(InterfaceData& interfaceData)
{ {
auto& vtable = interfaceData.vtable; auto& vtable = interfaceData.vtable;
@ -262,7 +267,7 @@ void Object::registerMethodsToVTable(const InterfaceData& interfaceData, std::ve
, methodData.outputArgs.c_str() , methodData.outputArgs.c_str()
, methodData.paramNames.c_str() , methodData.paramNames.c_str()
, &Object::sdbus_method_callback , &Object::sdbus_method_callback
, methodData.flags_.toSdBusMethodFlags() )); , methodData.flags.toSdBusMethodFlags() ));
} }
} }
@ -305,7 +310,7 @@ void Object::activateInterfaceVTable( const std::string& interfaceName
, InterfaceData& interfaceData , InterfaceData& interfaceData
, const std::vector<sd_bus_vtable>& vtable ) , const std::vector<sd_bus_vtable>& vtable )
{ {
interfaceData.slot = connection_.addObjectVTable(objectPath_, interfaceName, &vtable[0], this); interfaceData.slot = connection_.addObjectVTable(objectPath_, interfaceName, &vtable[0], &interfaceData);
} }
std::string Object::paramNamesToString(const std::vector<std::string>& paramNames) std::string Object::paramNamesToString(const std::vector<std::string>& paramNames)
@ -318,19 +323,19 @@ std::string Object::paramNamesToString(const std::vector<std::string>& paramName
int Object::sdbus_method_callback(sd_bus_message *sdbusMessage, void *userData, sd_bus_error *retError) int Object::sdbus_method_callback(sd_bus_message *sdbusMessage, void *userData, sd_bus_error *retError)
{ {
auto* object = static_cast<Object*>(userData); auto* interfaceData = static_cast<InterfaceData*>(userData);
assert(object != nullptr); assert(interfaceData != nullptr);
auto& object = interfaceData->object;
auto message = Message::Factory::create<MethodCall>(sdbusMessage, &object->connection_.getSdBusInterface()); auto message = Message::Factory::create<MethodCall>(sdbusMessage, &object.connection_.getSdBusInterface());
object->m_CurrentlyProcessedMessage.store(&message, std::memory_order_relaxed); object.m_CurrentlyProcessedMessage.store(&message, std::memory_order_relaxed);
SCOPE_EXIT SCOPE_EXIT
{ {
object->m_CurrentlyProcessedMessage.store(nullptr, std::memory_order_relaxed); object.m_CurrentlyProcessedMessage.store(nullptr, std::memory_order_relaxed);
}; };
// Note: The lookup can be optimized by using sorted vectors instead of associative containers auto& callback = interfaceData->methods[message.getMemberName()].callback;
auto& callback = object->interfaces_[message.getInterfaceName()].methods[message.getMemberName()].callback;
assert(callback); assert(callback);
try try
@ -347,17 +352,17 @@ int Object::sdbus_method_callback(sd_bus_message *sdbusMessage, void *userData,
int Object::sdbus_property_get_callback( sd_bus */*bus*/ int Object::sdbus_property_get_callback( sd_bus */*bus*/
, const char */*objectPath*/ , const char */*objectPath*/
, const char *interface , const char */*interface*/
, const char *property , const char *property
, sd_bus_message *sdbusReply , sd_bus_message *sdbusReply
, void *userData , void *userData
, sd_bus_error *retError ) , sd_bus_error *retError )
{ {
auto* object = static_cast<Object*>(userData); auto* interfaceData = static_cast<InterfaceData*>(userData);
assert(object != nullptr); assert(interfaceData != nullptr);
auto& object = interfaceData->object;
// Note: The lookup can be optimized by using sorted vectors instead of associative containers auto& callback = interfaceData->properties[property].getCallback;
auto& callback = object->interfaces_[interface].properties[property].getCallback;
// Getter can be empty - the case of "write-only" property // Getter can be empty - the case of "write-only" property
if (!callback) if (!callback)
{ {
@ -365,7 +370,7 @@ int Object::sdbus_property_get_callback( sd_bus */*bus*/
return 1; return 1;
} }
auto reply = Message::Factory::create<PropertyGetReply>(sdbusReply, &object->connection_.getSdBusInterface()); auto reply = Message::Factory::create<PropertyGetReply>(sdbusReply, &object.connection_.getSdBusInterface());
try try
{ {
@ -381,25 +386,25 @@ int Object::sdbus_property_get_callback( sd_bus */*bus*/
int Object::sdbus_property_set_callback( sd_bus */*bus*/ int Object::sdbus_property_set_callback( sd_bus */*bus*/
, const char */*objectPath*/ , const char */*objectPath*/
, const char *interface , const char */*interface*/
, const char *property , const char *property
, sd_bus_message *sdbusValue , sd_bus_message *sdbusValue
, void *userData , void *userData
, sd_bus_error *retError ) , sd_bus_error *retError )
{ {
auto* object = static_cast<Object*>(userData); auto* interfaceData = static_cast<InterfaceData*>(userData);
assert(object != nullptr); assert(interfaceData != nullptr);
auto& object = interfaceData->object;
// Note: The lookup can be optimized by using sorted vectors instead of associative containers auto& callback = interfaceData->properties[property].setCallback;
auto& callback = object->interfaces_[interface].properties[property].setCallback;
assert(callback); assert(callback);
auto value = Message::Factory::create<PropertySetCall>(sdbusValue, &object->connection_.getSdBusInterface()); auto value = Message::Factory::create<PropertySetCall>(sdbusValue, &object.connection_.getSdBusInterface());
object->m_CurrentlyProcessedMessage.store(&value, std::memory_order_relaxed); object.m_CurrentlyProcessedMessage.store(&value, std::memory_order_relaxed);
SCOPE_EXIT SCOPE_EXIT
{ {
object->m_CurrentlyProcessedMessage.store(nullptr, std::memory_order_relaxed); object.m_CurrentlyProcessedMessage.store(nullptr, std::memory_order_relaxed);
}; };
try try

8
src/Object.h
View File

@ -109,6 +109,8 @@ namespace sdbus::internal {
using InterfaceName = std::string; using InterfaceName = std::string;
struct InterfaceData struct InterfaceData
{ {
InterfaceData(Object& object) : object(object) {}
using MethodName = std::string; using MethodName = std::string;
struct MethodData struct MethodData
{ {
@ -116,7 +118,7 @@ namespace sdbus::internal {
const std::string outputArgs; const std::string outputArgs;
const std::string paramNames; const std::string paramNames;
method_callback callback; method_callback callback;
Flags flags_; Flags flags;
}; };
std::map<MethodName, MethodData> methods; std::map<MethodName, MethodData> methods;
using SignalName = std::string; using SignalName = std::string;
@ -138,10 +140,14 @@ namespace sdbus::internal {
std::map<PropertyName, PropertyData> properties; std::map<PropertyName, PropertyData> properties;
std::vector<sd_bus_vtable> vtable; std::vector<sd_bus_vtable> vtable;
Flags flags; Flags flags;
Object& object;
// This is intentionally the last member, because it must be destructed first,
// releasing callbacks above before the callbacks themselves are destructed.
SlotPtr slot; SlotPtr slot;
}; };
InterfaceData& getInterface(const std::string& interfaceName);
static const std::vector<sd_bus_vtable>& createInterfaceVTable(InterfaceData& interfaceData); static const std::vector<sd_bus_vtable>& createInterfaceVTable(InterfaceData& interfaceData);
static void registerMethodsToVTable(const InterfaceData& interfaceData, std::vector<sd_bus_vtable>& vtable); static void registerMethodsToVTable(const InterfaceData& interfaceData, std::vector<sd_bus_vtable>& vtable);
static void registerSignalsToVTable(const InterfaceData& interfaceData, std::vector<sd_bus_vtable>& vtable); static void registerSignalsToVTable(const InterfaceData& interfaceData, std::vector<sd_bus_vtable>& vtable);