Skip to content

Commit

Permalink
[COM] Release on QueryInterface with missing stub (rdkcentral#1576)
Browse files Browse the repository at this point in the history 
* [COM] Release on QueryInterface with missing stub

* Correct trace

---------

Co-authored-by: Pierre Wielders <[email protected]>
  • Loading branch information
@sebaszm @pwielders
sebaszm and pwielders authored Apr 21, 2024
1 parent 74874ef commit 3477427
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 45 deletions.
78 changes: 39 additions & 39 deletions Source/com/Administrator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,52 +293,52 @@ namespace RPC {

void Administrator::RegisterUnknownInterface(Core::ProxyType<Core::IPCChannel>& channel, Core::IUnknown* reference, const uint32_t id)
{
if (reference != nullptr) {
_adminLock.Lock();
ASSERT(reference != nullptr);

ReferenceMap::iterator index = _channelReferenceMap.find(channel->LinkId());
_adminLock.Lock();

if (index == _channelReferenceMap.end()) {
auto result = _channelReferenceMap.emplace(std::piecewise_construct,
std::forward_as_tuple(channel->LinkId()),
std::forward_as_tuple());
result.first->second.emplace_back(id, reference);
TRACE_L3("Registered interface %p(0x%08x).", reference, id);
} else {
// See that it does not already exists on this channel, no need to register
// it again!!!
std::list< RecoverySet >::iterator element(index->second.begin());
ReferenceMap::iterator index = _channelReferenceMap.find(channel->LinkId());

while ( (element != index->second.end()) && ((element->Id() != id) || (element->Unknown() != reference)) ) {
element++;
}
if (index == _channelReferenceMap.end()) {
auto result = _channelReferenceMap.emplace(std::piecewise_construct,
std::forward_as_tuple(channel->LinkId()),
std::forward_as_tuple());
result.first->second.emplace_back(id, reference);
TRACE_L3("Registered interface %p(0x%08x).", reference, id);
} else {
// See that it does not already exists on this channel, no need to register
// it again!!!
std::list< RecoverySet >::iterator element(index->second.begin());

if (element == index->second.end()) {
// Add this element to the list. We are referencing it now with a proxy on the other side..
index->second.emplace_back(id, reference);
TRACE_L3("Registered interface %p(0x%08x).", reference, id);
}
else {
// If this happens, it means that the interface we are trying to register, is already handed out, over the same channel.
// This means, that on the otherside (the receiving side) that will create a Proxy for this interface, finds this interface as well.
// Now two things can happen:
// 1) Everything is stable, when this call arrives on the otherside, the proxy is found, and the externalReferenceCount (the number
// of AddRefs the RemoteSide has on this Real Object is incremented by one).
// 2) Corner case, unlikely top happen, but we need to cater for it. If during the return of this reference, that Proxy on the otherside
// might reach the reference 0. That will, on that side, clear out the proxy. That will send a Release for that proxy to this side and
// that release will not kill the "real" object here becasue we have still a reference on the real object for this interface. When this
// interface reaches the other side, it will simply create a new proxy with an externalReference COunt of 1.
//
// However, if the connection dies and scenario 2 took place, and we did *not* reference count this cleanup map, this reference for the newly
// created proxy in step 2, is in case of a crash never released!!! So to avoid this scenario, we should also reference count the cleanup map
// interface entry here, than we are good to go, as long as the "dropReleases" count also ends up here :-)
TRACE_L3("Interface 0x%p(0x%08x) is already registered.", reference, id);
element->Increment();
}
while ( (element != index->second.end()) && ((element->Id() != id) || (element->Unknown() != reference)) ) {
element++;
}

_adminLock.Unlock();
if (element == index->second.end()) {
// Add this element to the list. We are referencing it now with a proxy on the other side..
index->second.emplace_back(id, reference);
TRACE_L3("Registered interface %p(0x%08x).", reference, id);
}
else {
// If this happens, it means that the interface we are trying to register, is already handed out, over the same channel.
// This means, that on the otherside (the receiving side) that will create a Proxy for this interface, finds this interface as well.
// Now two things can happen:
// 1) Everything is stable, when this call arrives on the otherside, the proxy is found, and the externalReferenceCount (the number
// of AddRefs the RemoteSide has on this Real Object is incremented by one).
// 2) Corner case, unlikely top happen, but we need to cater for it. If during the return of this reference, that Proxy on the otherside
// might reach the reference 0. That will, on that side, clear out the proxy. That will send a Release for that proxy to this side and
// that release will not kill the "real" object here becasue we have still a reference on the real object for this interface. When this
// interface reaches the other side, it will simply create a new proxy with an externalReference COunt of 1.
//
// However, if the connection dies and scenario 2 took place, and we did *not* reference count this cleanup map, this reference for the newly
// created proxy in step 2, is in case of a crash never released!!! So to avoid this scenario, we should also reference count the cleanup map
// interface entry here, than we are good to go, as long as the "dropReleases" count also ends up here :-)
TRACE_L3("Interface 0x%p(0x%08x) is already registered.", reference, id);
element->Increment();
}
}

_adminLock.Unlock();
}

Core::IUnknown* Administrator::Convert(void* rawImplementation, const uint32_t id)
Expand Down
20 changes: 16 additions & 4 deletions Source/com/Administrator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,13 +250,25 @@ namespace RPC {
// ----------------------------------------------------------------------------------------------------
// Stub method for entries that the Stub returns to the callee
template <typename ACTUALINTERFACE>
void RegisterInterface(Core::ProxyType<Core::IPCChannel>& channel, ACTUALINTERFACE* reference)
bool RegisterInterface(Core::ProxyType<Core::IPCChannel>& channel, ACTUALINTERFACE* reference)
{
RegisterInterface(channel, reference, ACTUALINTERFACE::ID);
return (RegisterInterface(channel, reference, ACTUALINTERFACE::ID));
}
void RegisterInterface(Core::ProxyType<Core::IPCChannel>& channel, const void* source, const uint32_t id)
bool RegisterInterface(Core::ProxyType<Core::IPCChannel>& channel, const void* source, const uint32_t id)
{
RegisterUnknownInterface(channel, Convert(const_cast<void*>(source), id), id);
bool result = false;

Core::IUnknown* converted = Convert(const_cast<void*>(source), id);

if (converted != nullptr) {
RegisterUnknownInterface(channel, converted, id);
result = true;
}
else {
TRACE_L1("Failed to find a Stub for interface 0x%08x!", id);
}

return (result);
}

void UnregisterInterface(Core::ProxyType<Core::IPCChannel>& channel, const Core::IUnknown* source, const uint32_t interfaceId, const uint32_t dropCount)
Expand Down
8 changes: 6 additions & 2 deletions Source/com/IUnknown.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,16 @@ namespace ProxyStub {
uint32_t newInterfaceId(reader.Number<uint32_t>());

void* newInterface = implementation->QueryInterface(newInterfaceId);
response.Number<Core::instance_id>(RPC::instance_cast<void*>(newInterface));

if (newInterface != nullptr) {
RPC::Administrator::Instance().RegisterInterface(channel, newInterface, newInterfaceId);
if (RPC::Administrator::Instance().RegisterInterface(channel, newInterface, newInterfaceId) == false) {
Convert(newInterface)->Release();
newInterface = nullptr;
}
}

response.Number<Core::instance_id>(RPC::instance_cast<void*>(newInterface));

break;
}
default: {
Expand Down

0 comments on commit 3477427

Please sign in to comment.