From 4d295519b64305ed8cf639fe0a48bdfc301c21e7 Mon Sep 17 00:00:00 2001
From: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com>
Date: Mon, 22 Feb 2016 10:24:52 -0500
Subject: [PATCH] videomanager: fix bad memory access
- remove videoManager global instance.
- allocate a VideoManager instance when Manager instance is created.
- add Manager::getVideoManager() method to get this instance.
- force getSignalHandlers() to be called before Manager instance creation.
All of this prevents VideoManager to access to signal handlers map,
already destroyed, causing a memory access violation.
Change-Id: I21ebff08fa8d49e797f99299de12dab023c37769
Tuleap: #413
---
src/client/videomanager.cpp | 45 +++++++++++++--------------
src/client/videomanager.h | 2 --
src/manager.cpp | 3 ++
src/manager.h | 8 +++--
src/media/video/video_rtp_session.cpp | 2 +-
src/ring_api.cpp | 6 ++++
src/sip/sipcall.cpp | 4 +--
7 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/src/client/videomanager.cpp b/src/client/videomanager.cpp
index 10f4b9b48e..16a8a52ccc 100644
--- a/src/client/videomanager.cpp
+++ b/src/client/videomanager.cpp
@@ -39,8 +39,6 @@
namespace DRing {
-using ring::videoManager;
-
void
registerVideoHandlers(const std::map<std::string,
std::shared_ptr<CallbackWrapperBase>>& handlers)
@@ -61,65 +59,65 @@ registerVideoHandlers(const std::map<std::string,
std::vector<std::string>
getDeviceList()
{
- return videoManager.videoDeviceMonitor.getDeviceList();
+ return ring::Manager::instance().getVideoManager().videoDeviceMonitor.getDeviceList();
}
VideoCapabilities
getCapabilities(const std::string& name)
{
- return videoManager.videoDeviceMonitor.getCapabilities(name);
+ return ring::Manager::instance().getVideoManager().videoDeviceMonitor.getCapabilities(name);
}
std::string
getDefaultDevice()
{
- return videoManager.videoDeviceMonitor.getDefaultDevice();
+ return ring::Manager::instance().getVideoManager().videoDeviceMonitor.getDefaultDevice();
}
void
setDefaultDevice(const std::string& name)
{
RING_DBG("Setting default device to %s", name.c_str());
- videoManager.videoDeviceMonitor.setDefaultDevice(name);
+ ring::Manager::instance().getVideoManager().videoDeviceMonitor.setDefaultDevice(name);
}
std::map<std::string, std::string>
getSettings(const std::string& name)
{
- return videoManager.videoDeviceMonitor.getSettings(name).to_map();
+ return ring::Manager::instance().getVideoManager().videoDeviceMonitor.getSettings(name).to_map();
}
void
applySettings(const std::string& name,
const std::map<std::string, std::string>& settings)
{
- videoManager.videoDeviceMonitor.applySettings(name, settings);
+ ring::Manager::instance().getVideoManager().videoDeviceMonitor.applySettings(name, settings);
}
void
startCamera()
{
- videoManager.videoPreview = ring::getVideoCamera();
- videoManager.started = switchToCamera();
+ ring::Manager::instance().getVideoManager().videoPreview = ring::getVideoCamera();
+ ring::Manager::instance().getVideoManager().started = switchToCamera();
}
void
stopCamera()
{
if (switchInput(""))
- videoManager.started = false;
- videoManager.videoPreview.reset();
+ ring::Manager::instance().getVideoManager().started = false;
+ ring::Manager::instance().getVideoManager().videoPreview.reset();
}
bool
switchInput(const std::string& resource)
{
if (auto call = ring::Manager::instance().getCurrentCall()) {
- // TODO remove this part when clients are updated to use CallManager::switchInput
+ // TODO remove this part when clients are updated to use Callring::Manager::switchInput
call->switchInput(resource);
return true;
} else {
- if (auto input = videoManager.videoInput.lock())
+ if (auto input = ring::Manager::instance().getVideoManager().videoInput.lock())
return input->switchInput(resource).valid();
RING_WARN("Video input not initialized");
}
@@ -129,13 +127,13 @@ switchInput(const std::string& resource)
bool
switchToCamera()
{
- return switchInput(videoManager.videoDeviceMonitor.getMRLForDefaultDevice());
+ return switchInput(ring::Manager::instance().getVideoManager().videoDeviceMonitor.getMRLForDefaultDevice());
}
bool
hasCameraStarted()
{
- return videoManager.started;
+ return ring::Manager::instance().getVideoManager().started;
}
void
@@ -151,13 +149,13 @@ registerSinkTarget(const std::string& sinkId, const SinkTarget& target)
void
addVideoDevice(const std::string &node)
{
- videoManager.videoDeviceMonitor.addDevice(node);
+ ring::Manager::instance().getVideoManager().videoDeviceMonitor.addDevice(node);
}
void
removeVideoDevice(const std::string &node)
{
- videoManager.videoDeviceMonitor.removeDevice(node);
+ ring::Manager::instance().getVideoManager().videoDeviceMonitor.removeDevice(node);
}
void*
@@ -181,24 +179,23 @@ releaseFrame(void* frame)
namespace ring {
-VideoManager videoManager;
-
std::shared_ptr<video::VideoFrameActiveWriter>
getVideoCamera()
{
- if (auto input = videoManager.videoInput.lock())
+ auto& vmgr = Manager::instance().getVideoManager();
+ if (auto input = vmgr.videoInput.lock())
return input;
- videoManager.started = false;
+ vmgr.started = false;
auto input = std::make_shared<video::VideoInput>();
- videoManager.videoInput = input;
+ vmgr.videoInput = input;
return input;
}
video::VideoDeviceMonitor&
getVideoDeviceMonitor()
{
- return videoManager.videoDeviceMonitor;
+ return Manager::instance().getVideoManager().videoDeviceMonitor;
}
} // namespace ring
diff --git a/src/client/videomanager.h b/src/client/videomanager.h
index 47ff88dff5..b9c0078097 100644
--- a/src/client/videomanager.h
+++ b/src/client/videomanager.h
@@ -51,8 +51,6 @@ struct VideoManager
std::atomic_bool started;
};
-extern VideoManager videoManager;
-
std::shared_ptr<video::VideoFrameActiveWriter> getVideoCamera();
video::VideoDeviceMonitor& getVideoDeviceMonitor();
diff --git a/src/manager.cpp b/src/manager.cpp
index 4a78c54824..eccb225949 100644
--- a/src/manager.cpp
+++ b/src/manager.cpp
@@ -213,6 +213,9 @@ Manager::Manager() :
, ringbufferpool_(new RingBufferPool)
, callFactory(), conferenceMap_()
, accountFactory_(), ice_tf_()
+#ifdef RING_VIDEO
+ , videoManager_(new VideoManager)
+#endif
{
// initialize random generator
// mt19937_64 should be seeded with 2 x 32 bits
diff --git a/src/manager.h b/src/manager.h
index 688c9a087c..735d38de00 100644
--- a/src/manager.h
+++ b/src/manager.h
@@ -66,6 +66,7 @@ class PluginManager;
class AudioFile;
class DTMF;
class RingBufferPool;
+class VideoManager;
/** To send multiple string */
typedef std::list<std::string> TokenList;
@@ -758,7 +759,6 @@ class Manager {
*/
std::vector<std::string> loadAccountOrder() const;
-
private:
void removeAccounts();
@@ -961,8 +961,8 @@ class Manager {
#ifdef RING_VIDEO
std::shared_ptr<video::SinkClient> createSinkClient(const std::string& id="", bool mixer=false);
-
std::shared_ptr<video::SinkClient> getSinkClient(const std::string& id);
+ VideoManager& getVideoManager() const { return *videoManager_; }
#endif // RING_VIDEO
private:
@@ -1012,6 +1012,10 @@ class Manager {
const std::map<std::string, std::string>& messages,
const std::string& from) const noexcept;
#endif
+
+#ifdef RING_VIDEO
+ std::unique_ptr<VideoManager> videoManager_;
+#endif
};
// Helper to install a callback to be called once by the main event loop
diff --git a/src/media/video/video_rtp_session.cpp b/src/media/video/video_rtp_session.cpp
index d00944e8da..49424a3a48 100644
--- a/src/media/video/video_rtp_session.cpp
+++ b/src/media/video/video_rtp_session.cpp
@@ -73,7 +73,7 @@ void VideoRtpSession::startSender()
if (not conference_) {
videoLocal_ = getVideoCamera();
- if (auto input = videoManager.videoInput.lock()) {
+ if (auto input = Manager::instance().getVideoManager().videoInput.lock()) {
auto newParams = input->switchInput(input_);
try {
if (newParams.valid() &&
diff --git a/src/ring_api.cpp b/src/ring_api.cpp
index d43eb3650c..e1fd4125ac 100644
--- a/src/ring_api.cpp
+++ b/src/ring_api.cpp
@@ -33,6 +33,7 @@
#include "callmanager_interface.h"
#include "configurationmanager_interface.h"
#include "presencemanager_interface.h"
+#include "client/ring_signal.h"
#ifdef RING_VIDEO
#include "client/videomanager.h"
@@ -46,6 +47,11 @@ init(enum InitFlag flags) noexcept
::setDebugMode(flags & DRING_FLAG_DEBUG);
::setConsoleLog(flags & DRING_FLAG_CONSOLE_LOG);
+ // Following function create a local static variable inside
+ // This var must have the same live as Manager.
+ // So we call it now to create this var.
+ ring::getSignalHandlers();
+
try {
// current implementation use static variable
return &ring::Manager::instance() != nullptr;
diff --git a/src/sip/sipcall.cpp b/src/sip/sipcall.cpp
index 1d2cfa8ef9..eb858de2db 100644
--- a/src/sip/sipcall.cpp
+++ b/src/sip/sipcall.cpp
@@ -119,7 +119,7 @@ SIPCall::SIPCall(SIPAccountBase& account, const std::string& id, Call::CallType
#ifdef RING_VIDEO
// The ID is used to associate video streams to calls
, videortp_(id, getVideoSettings())
- , videoInput_(videoManager.videoDeviceMonitor.getMRLForDefaultDevice())
+ , videoInput_(Manager::instance().getVideoManager().videoDeviceMonitor.getMRLForDefaultDevice())
#endif
, sdp_(new Sdp(id))
{
@@ -907,7 +907,7 @@ SIPCall::muteMedia(const std::string& mediaType, bool mute)
if (mute == isVideoMuted_) return;
RING_WARN("[call:%s] video muting %s", getCallId().c_str(), bool_to_str(mute));
isVideoMuted_ = mute;
- videoInput_ = isVideoMuted_ ? "" : videoManager.videoDeviceMonitor.getMRLForDefaultDevice();
+ videoInput_ = isVideoMuted_ ? "" : Manager::instance().getVideoManager().videoDeviceMonitor.getMRLForDefaultDevice();
DRing::switchInput(getCallId(), videoInput_);
emitSignal<DRing::CallSignal::VideoMuted>(getCallId(), isVideoMuted_);
#endif
--
GitLab