Skip to content

Commit

Permalink
Game add-ons: Fix segfault when opening game
Browse files Browse the repository at this point in the history
This fixes a segfault that occurs when moving an analog stick on game
open. The analog stick event is delivered while joysticks are being
connected/disconnected.
  • Loading branch information
garbear committed Jun 12, 2018
1 parent e6b2c79 commit 2eccda1
Show file tree
Hide file tree
Showing 9 changed files with 170 additions and 10 deletions.
14 changes: 13 additions & 1 deletion xbmc/games/addons/input/GameClientInput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "guilib/GUIWindowManager.h"
#include "guilib/WindowIDs.h"
#include "input/joysticks/JoystickTypes.h"
#include "peripherals/EventLockHandle.h"
#include "peripherals/Peripherals.h"
#include "threads/SingleLock.h"
#include "utils/log.h"
Expand Down Expand Up @@ -392,8 +393,11 @@ bool CGameClientInput::OpenJoystick(const std::string &portAddress, const Contro

if (bSuccess)
{
PERIPHERALS::EventLockHandlePtr lock = CServiceBroker::GetPeripherals().RegisterEventLock();

m_joysticks[portAddress].reset(new CGameClientJoystick(m_gameClient, portAddress, controller, m_struct.toAddon));
ProcessJoysticks();

return true;
}

Expand All @@ -407,7 +411,12 @@ void CGameClientInput::CloseJoystick(const std::string &portAddress)
{
std::unique_ptr<CGameClientJoystick> joystick = std::move(it->second);
m_joysticks.erase(it);
ProcessJoysticks();
{
PERIPHERALS::EventLockHandlePtr lock = CServiceBroker::GetPeripherals().RegisterEventLock();

ProcessJoysticks();
joystick.reset();
}
}

{
Expand Down Expand Up @@ -467,7 +476,10 @@ void CGameClientInput::Notify(const Observable& obs, const ObservableMessage msg
{
case ObservableMessagePeripheralsChanged:
{
PERIPHERALS::EventLockHandlePtr lock = CServiceBroker::GetPeripherals().RegisterEventLock();

ProcessJoysticks();

break;
}
default:
Expand Down
6 changes: 4 additions & 2 deletions xbmc/peripherals/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
set(SOURCES EventPollHandle.cpp
set(SOURCES EventLockHandle.cpp
EventPollHandle.cpp
EventScanner.cpp
Peripherals.cpp)

set(HEADERS EventPollHandle.h
set(HEADERS EventLockHandle.h
EventPollHandle.h
EventScanner.h
Peripherals.h
PeripheralTypes.h)
Expand Down
33 changes: 33 additions & 0 deletions xbmc/peripherals/EventLockHandle.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright (C) 2018 Team Kodi
* http://kodi.tv
*
* This Program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2, or (at your option)
* any later version.
*
* This Program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this Program; see the file COPYING. If not, see
* <http://www.gnu.org/licenses/>.
*
*/

#include "EventLockHandle.h"

using namespace PERIPHERALS;

CEventLockHandle::CEventLockHandle(IEventLockCallback &callback) :
m_callback(callback)
{
}

CEventLockHandle::~CEventLockHandle(void)
{
m_callback.ReleaseLock(*this);
}
59 changes: 59 additions & 0 deletions xbmc/peripherals/EventLockHandle.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright (C) 2018 Team Kodi
* http://kodi.tv
*
* This Program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2, or (at your option)
* any later version.
*
* This Program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this Program; see the file COPYING. If not, see
* <http://www.gnu.org/licenses/>.
*
*/
#pragma once

namespace PERIPHERALS
{
class CEventLockHandle;

/*!
* \brief Callback implemented by event scanner
*/
class IEventLockCallback
{
public:
virtual ~IEventLockCallback(void) = default;

virtual void ReleaseLock(CEventLockHandle *handle) = 0;
};

/*!
* \brief Handle returned by the event scanner to disable event processing
*
* When held, this disables event processing.
*/
class CEventLockHandle
{
public:
/*!
* \brief Create an event lock handle
*/
CEventLockHandle(IEventLockCallback &callback);

/*!
* \brief Handle is automatically released when this class is destructed
*/
~CEventLockHandle(void);

private:
// Construction parameters
IEventLockCallback &m_callback;
};
}
40 changes: 34 additions & 6 deletions xbmc/peripherals/EventScanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ EventPollHandlePtr CEventScanner::RegisterPollHandle()
EventPollHandlePtr handle(new CEventPollHandle(this));

{
CSingleLock lock(m_mutex);
CSingleLock lock(m_handleMutex);
m_activeHandles.insert(handle.get());
}

Expand All @@ -72,7 +72,7 @@ EventPollHandlePtr CEventScanner::RegisterPollHandle()
void CEventScanner::Activate(CEventPollHandle* handle)
{
{
CSingleLock lock(m_mutex);
CSingleLock lock(m_handleMutex);
m_activeHandles.insert(handle);
}

Expand All @@ -82,7 +82,7 @@ void CEventScanner::Activate(CEventPollHandle* handle)
void CEventScanner::Deactivate(CEventPollHandle* handle)
{
{
CSingleLock lock(m_mutex);
CSingleLock lock(m_handleMutex);
m_activeHandles.erase(handle);
}

Expand All @@ -108,20 +108,48 @@ void CEventScanner::HandleEvents(bool bWait)
void CEventScanner::Release(CEventPollHandle* handle)
{
{
CSingleLock lock(m_mutex);
CSingleLock lock(m_handleMutex);
m_activeHandles.erase(handle);
}

CLog::Log(LOGDEBUG, "PERIPHERALS: Event poll handle released");
}

EventLockHandlePtr CEventScanner::RegisterLock()
{
EventLockHandlePtr handle(new CEventLockHandle(*this));

{
CSingleLock lock(m_lockMutex);
m_activeLocks.insert(handle.get());
}

CLog::Log(LOGDEBUG, "PERIPHERALS: Event lock handle registered");

return handle;
}

void CEventScanner::ReleaseLock(CEventLockHandle &handle)
{
{
CSingleLock lock(m_lockMutex);
m_activeLocks.erase(&handle);
}

CLog::Log(LOGDEBUG, "PERIPHERALS: Event lock handle released");
}

void CEventScanner::Process(void)
{
double nextScanMs = static_cast<double>(SystemClockMillis());

while (!m_bStop)
{
m_callback->ProcessEvents();
{
CSingleLock lock(m_lockMutex);
if (m_activeLocks.empty())
m_callback->ProcessEvents();
}

m_scanFinishedEvent.Set();

Expand All @@ -147,7 +175,7 @@ double CEventScanner::GetScanIntervalMs() const
bool bHasActiveHandle;

{
CSingleLock lock(m_mutex);
CSingleLock lock(m_handleMutex);
bHasActiveHandle = !m_activeHandles.empty();
}

Expand Down
14 changes: 13 additions & 1 deletion xbmc/peripherals/EventScanner.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <set>

#include "EventLockHandle.h"
#include "EventPollHandle.h"
#include "PeripheralTypes.h"
#include "threads/CriticalSection.h"
Expand All @@ -45,6 +46,7 @@ namespace PERIPHERALS
* input is handled by registering for a polling handle.
*/
class CEventScanner : public IEventPollCallback,
public IEventLockCallback,
protected CThread
{
public:
Expand All @@ -57,12 +59,20 @@ namespace PERIPHERALS

EventPollHandlePtr RegisterPollHandle();

/*!
* \brief Acquire a lock that prevents event processing while held
*/
EventLockHandlePtr RegisterLock();

// implementation of IEventPollCallback
void Activate(CEventPollHandle *handle) override;
void Deactivate(CEventPollHandle *handle) override;
void HandleEvents(bool bWait) override;
void Release(CEventPollHandle *handle) override;

// implementation of IEventLockCallback
void ReleaseLock(CEventLockHandle *handle) override;

protected:
// implementation of CThread
void Process(void) override;
Expand All @@ -72,9 +82,11 @@ namespace PERIPHERALS

IEventScannerCallback* const m_callback;
std::set<void*> m_activeHandles;
std::set<void*> m_activeLocks;
CEvent m_scanEvent;
CEvent m_scanFinishedEvent;
CCriticalSection m_mutex;
CCriticalSection m_handleMutex;
CCriticalSection m_lockMutex;
CCriticalSection m_pollMutex; // Prevent two poll handles from polling at once
};
}
3 changes: 3 additions & 0 deletions xbmc/peripherals/PeripheralTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ namespace PERIPHERALS
class CEventPollHandle;
typedef std::unique_ptr<CEventPollHandle> EventPollHandlePtr;

class CEventLockHandle;
using EventLockHandlePtr = std::unique_ptr<CEventLockHandle>;

struct PeripheralID
{
int m_iVendorId;
Expand Down
5 changes: 5 additions & 0 deletions xbmc/peripherals/Peripherals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,11 @@ bool CPeripherals::GetNextKeypress(float frameTime, CKey &key)
return false;
}

EventLockHandlePtr CPeripherals::RegisterEventLock()
{
return m_eventScanner.RegisterLock();
}

void CPeripherals::OnUserNotification()
{
if (!CServiceBroker::GetSettings().GetBool(CSettings::SETTING_INPUT_RUMBLENOTIFY))
Expand Down
6 changes: 6 additions & 0 deletions xbmc/peripherals/Peripherals.h
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,12 @@ namespace PERIPHERALS
*/
EventPollHandlePtr RegisterEventPoller() { return m_eventScanner.RegisterPollHandle(); }

/*!
* @brief Register with the event scanner to disable event processing
* @return A handle that unregisters itself when expired
*/
EventLockHandlePtr RegisterEventLock();

/*!
*
*/
Expand Down

0 comments on commit 2eccda1

Please sign in to comment.