Skip to content

Commit

Permalink
[CONSOLE] Console redirect should not be handled by the Thunder Resou…
Browse files Browse the repository at this point in the history 
…rceMonitor. (#1494)

As the Console stream redirecting is writing data to filebuffers, the file buffers might
block as the buffer is full and first nees to be flushed. However if this happens while
code is being executed on the ResourceMonitor thread it create a clissical deadlock.

This use-case was observed by TRACE_LX macros that use a printf (to stderr) to write data
to the console. In case of redirection, this is done to the intermediate file descriptor.
If this intermediate file descriptor can not continue to write (as the buffer is full) the
printf (to stderr) will block, witing for the data to be read so new buffer space becomes
avaialble. However the readin of this data has to be doen by this ResourceMonitor thread
which is currently blocked by this printf.

To avoid this classical deadlock, the StreamRedirectType template now uses its own
instantiation of the ResourceMonitor. As this ResourceMonitor will only be used by (best
case) 2 descriptors (stdout and stderr) enhanced the template to control the number
of descriptors it expects and the Stack size required by these threads.
  • Loading branch information
@pwielders
pwielders authored Jan 9, 2024
1 parent a446bfd commit da1d94a
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 94 deletions.
166 changes: 80 additions & 86 deletions Source/core/ResourceMonitor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@
* limitations under the License.
*/

#ifndef RESOURCE_MONITOR_TYPE_H
#define RESOURCE_MONITOR_TYPE_H
#pragma once

#include "Module.h"
#include "Portability.h"
Expand All @@ -42,24 +41,22 @@ namespace Core {
virtual void Handle(const uint16_t events) = 0;
};

template <typename RESOURCE, typename WATCHDOG>
template <typename RESOURCE, typename WATCHDOG, const uint32_t STACK_SIZE, const uint8_t RESOURCE_SLOTS>
class ResourceMonitorType {
private:
static constexpr uint8_t FileDescriptorAllocation = 32;

typedef ResourceMonitorType<RESOURCE, WATCHDOG> Parent;

ResourceMonitorType(const ResourceMonitorType&) = delete;
ResourceMonitorType& operator=(const ResourceMonitorType&) = delete;
using Parent = ResourceMonitorType<RESOURCE, WATCHDOG, STACK_SIZE, RESOURCE_SLOTS>;
using Resources = std::vector<RESOURCE*>;

class MonitorWorker : public Core::Thread {
private:
public:
MonitorWorker() = delete;
MonitorWorker(MonitorWorker&&) = delete;
MonitorWorker(const MonitorWorker&) = delete;
MonitorWorker& operator=(MonitorWorker&&) = delete;
MonitorWorker& operator=(const MonitorWorker&) = delete;

public:
MonitorWorker(Parent& parent)
: Core::Thread(Thread::DefaultStackSize(), parent.Name())
: Core::Thread(STACK_SIZE == 0 ? Thread::DefaultStackSize() : STACK_SIZE, parent.Name())
, _parent(parent)
{
Thread::Init();
Expand All @@ -72,13 +69,13 @@ namespace Core {
}

public:
#ifdef __LINUX__
virtual uint32_t Initialize()
#ifdef __LINUX__
uint32_t Initialize() override
{
return ((Thread::Initialize() == Core::ERROR_NONE) && (_parent.Initialize() == Core::ERROR_NONE) ? Core::ERROR_NONE : Core::ERROR_UNAVAILABLE);
}
#endif
virtual uint32_t Worker()
#endif
uint32_t Worker() override
{
return (_parent.Worker());
}
Expand All @@ -97,33 +94,37 @@ namespace Core {
};

public:
ResourceMonitorType(ResourceMonitorType&&) = delete;
ResourceMonitorType(const ResourceMonitorType&) = delete;
ResourceMonitorType& operator=(ResourceMonitorType&&) = delete;
ResourceMonitorType& operator=(const ResourceMonitorType&) = delete;

ResourceMonitorType()
: _monitor(nullptr)
, _adminLock()
, _resourceList()
, _monitorRuns(0)
, _name(_T("Monitor::") + ClassNameOnly(typeid(RESOURCE).name()).Text())
, _watchDog(1024 * 512, _name.c_str())
#ifdef __WINDOWS__
#ifdef __WINDOWS__
, _action(WSACreateEvent())
#else
, _descriptorArrayLength(FileDescriptorAllocation)
, _descriptorArray(static_cast<struct pollfd*>(::malloc(sizeof(::pollfd) * (_descriptorArrayLength + 1))))
#else
, _descriptorArrayLength(RESOURCE_SLOTS)
, _descriptorArray(static_cast<struct pollfd*>(::malloc(sizeof(::pollfd) * (RESOURCE_SLOTS + 1))))
, _signalDescriptor(-1)
#endif
#endif
{
}

~ResourceMonitorType()
{

#ifdef __DEBUG__
#ifdef __DEBUG__
// All resources should be gone !!!
for (const auto& resource : _resourceList) {
TRACE_L1("Resource name: %s", typeid(resource).name());
ASSERT(resource == nullptr);
}
#endif
#endif

if (_monitor != nullptr) {

Expand All @@ -140,15 +141,15 @@ namespace Core {
delete _monitor;
}

#ifdef __LINUX__
#ifdef __LINUX__
::free(_descriptorArray);
if (_signalDescriptor != -1) {
::close(_signalDescriptor);
}
#endif
#ifdef __WINDOWS__
#endif
#ifdef __WINDOWS__
WSACloseEvent(_action);
#endif
#endif
}

public:
Expand All @@ -174,7 +175,7 @@ namespace Core {

_adminLock.Lock();

typename std::list<RESOURCE*>::const_iterator index(_resourceList.cbegin());
typename Resources::const_iterator index(_resourceList.cbegin());
while ( (count != 0) && (index != _resourceList.cend()) ) { count--; index++; }

bool found = (index != _resourceList.cend());
Expand All @@ -183,7 +184,7 @@ namespace Core {
info.descriptor = (*index)->Descriptor();
info.classname = typeid(*(*index)).name();

#ifdef __LINUX__
#ifdef __LINUX__
info.monitor = _descriptorArray[position + 1].events;
info.events = _descriptorArray[position + 1].revents;

Expand All @@ -192,12 +193,12 @@ namespace Core {

size_t len = readlink(procfn, info.filename, sizeof(info.filename) - 1);
info.filename[len] = '\0';
#endif
#ifdef __WINDOWS__
#endif
#ifdef __WINDOWS__
info.monitor = 0;
info.events = 0;
info.filename[0] = '\0';
#endif
#endif
}

_adminLock.Unlock();
Expand All @@ -216,7 +217,7 @@ namespace Core {
if (_resourceList.size() == 1) {
if (_monitor == nullptr) {
_monitor = new MonitorWorker(*this);

_monitorRuns = 0;
// Wait till we are at least initialized
_monitor->Wait(Thread::BLOCKED | Thread::STOPPED);
}
Expand All @@ -233,7 +234,7 @@ namespace Core {
_adminLock.Lock();

// Make sure this entry does not exist, only register resources once !!!
typename std::list<RESOURCE*>::iterator index(std::find(_resourceList.begin(), _resourceList.end(), &resource));
typename Resources::iterator index(std::find(_resourceList.begin(), _resourceList.end(), &resource));

if (index != _resourceList.end()) {
*index = nullptr;
Expand All @@ -244,21 +245,20 @@ namespace Core {
}
inline void Break()
{

ASSERT(_monitor != nullptr);

#ifdef __APPLE__
#ifdef __APPLE__
int data = 0;
::sendto(_signalDescriptor
& data,
sizeof(data), 0,
_signalNode,
_signalNode.Size());
#elif defined(__LINUX__)
#elif defined(__LINUX__)
_monitor->Signal(SIGUSR2);
#elif defined(__WINDOWS__)
#elif defined(__WINDOWS__)
::WSASetEvent(_action);
#endif
#endif
};

private:
Expand Down Expand Up @@ -292,11 +292,10 @@ namespace Core {
{
}

public:
#ifdef __LINUX__
#ifdef __LINUX__
uint32_t Initialize()
{
#ifdef __APPLE__
#ifdef __APPLE__

if ((_signalDescriptor = ::socket(AF_UNIX, SOCK_DGRAM, 0)) == -1) {
TRACE_L1("Error on creating socket SOCKET. Error %d", errno);
Expand All @@ -313,7 +312,7 @@ namespace Core {
}
}

#else
#else

sigset_t sigset;

Expand All @@ -330,7 +329,7 @@ namespace Core {
/* Create the signalfd */
_signalDescriptor = signalfd(-1, &sigset, SFD_CLOEXEC);

#endif
#endif

ASSERT(_signalDescriptor != -1);

Expand All @@ -340,9 +339,7 @@ namespace Core {

return (_signalDescriptor != -1 ? Core::ERROR_NONE : Core::ERROR_UNAVAILABLE);
}
#endif

#ifdef __LINUX__
uint32_t Worker()
{
uint32_t delay = 0;
Expand All @@ -354,7 +351,7 @@ namespace Core {

// Do we have enough space to allocate all file descriptors ?
if ((_resourceList.size() + 1) > _descriptorArrayLength) {
_descriptorArrayLength = ((((_resourceList.size() + 1) / FileDescriptorAllocation) + 1) * FileDescriptorAllocation);
_descriptorArrayLength = ((((_resourceList.size() + 1) / RESOURCE_SLOTS) + 1) * RESOURCE_SLOTS);

::free(_descriptorArray);

Expand All @@ -367,7 +364,7 @@ namespace Core {
}

int filledFileDescriptors = 1;
typename std::list<RESOURCE*>::iterator index = _resourceList.begin();
typename Resources::iterator index = _resourceList.begin();

// Fill in all entries required/updated..
while (index != _resourceList.end()) {
Expand Down Expand Up @@ -397,12 +394,12 @@ namespace Core {
TRACE_L1("poll failed with error <%d>", errno);

} else if (_descriptorArray[0].revents & POLLIN) {
#ifdef __APPLE__
#ifdef __APPLE__
int info;
#else
#else
/* We have a valid signal, read the info from the fd */
struct signalfd_siginfo info;
#endif
#endif
uint32_t VARIABLE_IS_NOT_USED bytes = read(_signalDescriptor, &info, sizeof(info));
ASSERT(bytes == sizeof(info) || bytes == 0);
}
Expand Down Expand Up @@ -446,13 +443,13 @@ namespace Core {

return (delay);
}
#endif
#endif

#ifdef __WINDOWS__
#ifdef __WINDOWS__
uint32_t Worker()
{
uint32_t delay = 0;
typename std::list<RESOURCE*>::iterator index;
typename Resources::iterator index;

_monitorRuns++;

Expand Down Expand Up @@ -520,43 +517,41 @@ namespace Core {

return (delay);
}
#endif
#endif

private:
MonitorWorker* _monitor;
mutable Core::CriticalSection _adminLock;
std::list<RESOURCE*> _resourceList;
Resources _resourceList;
uint32_t _monitorRuns;
string _name;
WATCHDOG _watchDog;

#ifdef __LINUX__
#ifdef __LINUX__
uint32_t _descriptorArrayLength;
struct ::pollfd* _descriptorArray;
int _signalDescriptor;
#endif
#endif

#ifdef __WINDOWS__
#ifdef __WINDOWS__
HANDLE _action;
#endif
#ifdef __APPLE__
#endif

#ifdef __APPLE__
Core::NodeId _signalNode;
#endif
#endif
};

#ifdef WATCHDOG_ENABLED
#ifdef WATCHDOG_ENABLED
class ResourceMonitorHandler {
private:
public:
ResourceMonitorHandler(ResourceMonitorHandler&& rhs) = delete;
ResourceMonitorHandler(const ResourceMonitorHandler& rhs) = delete;
ResourceMonitorHandler& operator=(ResourceMonitorHandler&& rhs) = delete;
ResourceMonitorHandler& operator=(const ResourceMonitorHandler& rhs) = delete;

public:
ResourceMonitorHandler()
{
}
~ResourceMonitorHandler()
{
}
ResourceMonitorHandler() = default;
~ResourceMonitorHandler() = default;

public:
uint32_t Expired()
Expand All @@ -566,27 +561,26 @@ namespace Core {
}
};

typedef ResourceMonitorType<IResource, WatchDogType<ResourceMonitorHandler>> ResourceMonitorBase;
#else
typedef ResourceMonitorType<IResource, Void> ResourceMonitorBase;
#endif
using ResourceMonitorBase = ResourceMonitorType<IResource, WatchDogType<ResourceMonitorHandler>, 0, 32> ;
#else
using ResourceMonitorBase = ResourceMonitorType<IResource, Void, 0, 32>;
#endif

class EXTERNAL ResourceMonitor : public ResourceMonitorBase {
private:
ResourceMonitor()
: ResourceMonitorBase()
{
}
ResourceMonitor(const ResourceMonitor&) = delete;
ResourceMonitor& operator=(const ResourceMonitor&) = delete;
friend SingletonType<ResourceMonitor>;

friend class SingletonType<ResourceMonitor>;
ResourceMonitor() = default;

public:
ResourceMonitor(ResourceMonitor&&) = delete;
ResourceMonitor(const ResourceMonitor&) = delete;
ResourceMonitor& operator=(ResourceMonitor&&) = delete;
ResourceMonitor& operator=(const ResourceMonitor&) = delete;

static ResourceMonitor& Instance();
~ResourceMonitor() {}

~ResourceMonitor() = default;
};
}
} // namespace WPEFramework::Core

#endif // RESOURCE_MONITOR_TYPE_H
Loading

0 comments on commit da1d94a

Please sign in to comment.