Coverity- High priority issue fixes

Summary: Fix for lock, missing_lock, uninitialized fields and uncaught
exception
Type: Fix
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-17892

Author: trupthi1403

common/source/LinuxUtils.cpp

@@ -19,6 +19,7 @@
 
 #include "LinuxUtils.h"
 #include "RialtoCommonLogging.h"
+#include <cstdint>
 #include <grp.h>
 #include <pwd.h>
 #include <sys/stat.h>
@@ -32,8 +33,8 @@ constexpr gid_t kNoGroupChange = -1; // -1 means chown() won't change the group
 uid_t getFileOwnerId(const std::string &fileOwner)
 {
     uid_t ownerId = kNoOwnerChange;
-    // sysconf returns long; -1 on error. Store as long to avoid unsigned conversion issues.
-    const long bufferSizeLong = sysconf(_SC_GETPW_R_SIZE_MAX);
+    // sysconf returns long; -1 on error. Store as int64_t to avoid unsigned conversion issues.
+    const int64_t bufferSizeLong = sysconf(_SC_GETPW_R_SIZE_MAX);
     if (!fileOwner.empty() && bufferSizeLong > 0)
     {
         const size_t kBufferSize = static_cast<size_t>(bufferSizeLong);
@@ -57,8 +58,8 @@ uid_t getFileOwnerId(const std::string &fileOwner)
 gid_t getFileGroupId(const std::string &fileGroup)
 {
     gid_t groupId = kNoGroupChange;
-    // sysconf returns long; -1 on error. Store as long to avoid unsigned conversion issues.
-    const long bufferSizeLong = sysconf(_SC_GETPW_R_SIZE_MAX);
+    // sysconf returns long; -1 on error. Store as int64_t to avoid unsigned conversion issues.
+    const int64_t bufferSizeLong = sysconf(_SC_GETGR_R_SIZE_MAX);
     if (!fileGroup.empty() && bufferSizeLong > 0)
     {
         const size_t kBufferSize = static_cast<size_t>(bufferSizeLong);

ipc/client/source/IpcChannelImpl.cpp

@@ -765,26 +765,27 @@ void ChannelImpl::processErrorFromServer(const transport::MethodCallError &error
 {
     RIALTO_IPC_LOG_DEBUG("processing error from server");
 
-    std::unique_lock<std::mutex> locker(m_lock);
-
-    // find the original request
+    MethodCall methodCall;
     const uint64_t kSerialId = error.reply_id();
-    auto it = m_methodCalls.find(kSerialId);
-    if (it == m_methodCalls.end())
+
     {
-        RIALTO_IPC_LOG_ERROR("failed to find request for received reply with id %" PRIu64 "", error.reply_id());
-        return;
-    }
+        std::unique_lock<std::mutex> locker(m_lock);
 
-    // take the method call and remove from the map of outstanding calls
-    MethodCall methodCall = it->second;
-    m_methodCalls.erase(it);
+        // find the original request
+        auto it = m_methodCalls.find(kSerialId);
+        if (it == m_methodCalls.end())
+        {
+            RIALTO_IPC_LOG_ERROR("failed to find request for received reply with id %" PRIu64 "", error.reply_id());
+            return;
+        }
 
-    // update the timeout timer now a method call has been processed
-    updateTimeoutTimer();
+        // take the method call and remove from the map of outstanding calls
+        methodCall = it->second;
+        m_methodCalls.erase(it);
 
-    // can now drop the lock
-    locker.unlock();
+        // update the timeout timer now a method call has been processed
+        updateTimeoutTimer();
+    }
 
     RIALTO_IPC_LOG_DEBUG("error{ serial %" PRIu64 " } - %s", kSerialId, error.error_reason().c_str());
 

ipc/common/source/NamedSocket.cpp

@@ -19,6 +19,7 @@
 
 #include "NamedSocket.h"
 #include "IpcLogging.h"
+#include <cstdint>
 #include <grp.h>
 #include <pwd.h>
 #include <stdexcept>
@@ -268,8 +269,8 @@ bool NamedSocket::getSocketLock()
 uid_t NamedSocket::getSocketOwnerId(const std::string &socketOwner) const
 {
     uid_t ownerId = kNoOwnerChange;
-    // sysconf returns long; -1 on error. Store as long to avoid unsigned conversion issues.
-    const long bufferSizeLong = sysconf(_SC_GETPW_R_SIZE_MAX);
+    // sysconf returns long; -1 on error. Store as int64_t to avoid unsigned conversion issues.
+    const int64_t bufferSizeLong = sysconf(_SC_GETPW_R_SIZE_MAX);
     if (!socketOwner.empty() && bufferSizeLong > 0)
     {
         const size_t kBufferSize = static_cast<size_t>(bufferSizeLong);
@@ -293,8 +294,8 @@ uid_t NamedSocket::getSocketOwnerId(const std::string &socketOwner) const
 gid_t NamedSocket::getSocketGroupId(const std::string &socketGroup) const
 {
     gid_t groupId = kNoGroupChange;
-    // sysconf returns long; -1 on error. Store as long to avoid unsigned conversion issues.
-    const long bufferSizeLong = sysconf(_SC_GETPW_R_SIZE_MAX);
+    // sysconf returns long; -1 on error. Store as int64_t to avoid unsigned conversion issues.
+    const int64_t bufferSizeLong = sysconf(_SC_GETGR_R_SIZE_MAX);
     if (!socketGroup.empty() && bufferSizeLong > 0)
     {
         const size_t kBufferSize = static_cast<size_t>(bufferSizeLong);

ipc/server/source/IpcServerImpl.cpp

@@ -616,37 +616,42 @@ void ServerImpl::processNewConnection(uint64_t socketId)
 {
     RIALTO_IPC_LOG_DEBUG("processing new connection");
 
-    std::unique_lock<std::mutex> socketLocker(m_socketsLock);
+    int clientSock;
+    std::string sockPath;
+    std::function<void(const std::shared_ptr<IClient> &)> connectedCb;
+    std::function<void(const std::shared_ptr<IClient> &)> disconnectedCb;
 
-    // find matching socket object
-    auto it = m_sockets.find(socketId);
-    if (it == m_sockets.end())
     {
-        RIALTO_IPC_LOG_ERROR("failed to find listening socket with id %" PRIu64, socketId);
-        return;
-    }
+        std::unique_lock<std::mutex> socketLocker(m_socketsLock);
 
-    const Socket &kSocket = it->second;
+        // find matching socket object
+        auto it = m_sockets.find(socketId);
+        if (it == m_sockets.end())
+        {
+            RIALTO_IPC_LOG_ERROR("failed to find listening socket with id %" PRIu64, socketId);
+            return;
+        }
 
-    // accept the connection from the client
-    struct sockaddr clientAddr = {0};
-    socklen_t clientAddrLen = sizeof(clientAddr);
+        const Socket &kSocket = it->second;
 
-    int clientSock = accept4(kSocket.sockFd, &clientAddr, &clientAddrLen, SOCK_NONBLOCK | SOCK_CLOEXEC);
-    if (clientSock < 0)
-    {
-        RIALTO_IPC_LOG_SYS_ERROR(errno, "failed to accept client connection");
-        return;
-    }
+        // accept the connection from the client
+        struct sockaddr clientAddr = {0};
+        socklen_t clientAddrLen = sizeof(clientAddr);
 
-    const std::string kSockPath = kSocket.sockPath;
-    std::function<void(const std::shared_ptr<IClient> &)> connectedCb = kSocket.connectedCb;
-    std::function<void(const std::shared_ptr<IClient> &)> disconnectedCb = kSocket.disconnectedCb;
+        clientSock = accept4(kSocket.sockFd, &clientAddr, &clientAddrLen, SOCK_NONBLOCK | SOCK_CLOEXEC);
+        if (clientSock < 0)
+        {
+            RIALTO_IPC_LOG_SYS_ERROR(errno, "failed to accept client connection");
+            return;
+        }
 
-    socketLocker.unlock();
+        sockPath = kSocket.sockPath;
+        connectedCb = kSocket.connectedCb;
+        disconnectedCb = kSocket.disconnectedCb;
+    }
 
     // attempt to add the socket to the client list
-    auto client = addClientSocket(clientSock, kSockPath, std::move(disconnectedCb));
+    auto client = addClientSocket(clientSock, sockPath, std::move(disconnectedCb));
     if (!client)
     {
         close(clientSock);
@@ -736,31 +741,34 @@ static std::vector<FileDescriptor> readMessageFds(const struct msghdr *msg, size
  */
 void ServerImpl::processClientSocket(uint64_t clientId, unsigned events)
 {
-    // take the lock while accessing the client list
-    std::unique_lock<std::mutex> locker(m_clientsLock);
+    int sockFd;
+    std::shared_ptr<ClientImpl> clientObj;
 
-    auto it = m_clients.find(clientId);
-    if (it == m_clients.end())
+    // take the lock while accessing the client list
     {
-        // should never happen
-        RIALTO_IPC_LOG_ERROR("received an event from a socket with no matching client");
-        return;
-    }
+        std::unique_lock<std::mutex> locker(m_clientsLock);
 
-    // check if the client is marked for closure, if so then just ignore the data
-    if (m_condemnedClients.count(clientId) != 0)
-    {
-        return;
-    }
+        auto it = m_clients.find(clientId);
+        if (it == m_clients.end())
+        {
+            // should never happen
+            RIALTO_IPC_LOG_ERROR("received an event from a socket with no matching client");
+            return;
+        }
 
-    // get the socket that corresponds to the client connection
-    const int kSockFd = it->second.sock;
+        // check if the client is marked for closure, if so then just ignore the data
+        if (m_condemnedClients.count(clientId) != 0)
+        {
+            return;
+        }
 
-    // get the client object
-    std::shared_ptr<ClientImpl> clientObj = it->second.client;
+        // get the socket that corresponds to the client connection
+        sockFd = it->second.sock;
 
-    // can safely release the lock now we have the clientId and client object
-    locker.unlock();
+        // get the client object
+        clientObj = it->second.client;
+    }
+    // lock is released here automatically
 
     // if there was an error disconnect the socket
     if (events & EPOLLERR)
@@ -786,7 +794,7 @@ void ServerImpl::processClientSocket(uint64_t clientId, unsigned events)
             msg.msg_controllen = sizeof(m_recvCtrlBuf);
 
             // read one message
-            ssize_t rd = TEMP_FAILURE_RETRY(recvmsg(kSockFd, &msg, MSG_CMSG_CLOEXEC));
+            ssize_t rd = TEMP_FAILURE_RETRY(recvmsg(sockFd, &msg, MSG_CMSG_CLOEXEC));
             if (rd < 0)
             {
                 if (errno != EWOULDBLOCK)
@@ -1394,21 +1402,22 @@ bool ServerImpl::sendEvent(uint64_t clientId, const std::shared_ptr<google::prot
     }
 
     // finally, take the lock (so the socket is not closed beneath us) and send the reply
-    std::unique_lock<std::mutex> locker(m_clientsLock);
-
-    auto it = m_clients.find(clientId);
-    if (it == m_clients.end() || it->second.sock < 0)
-    {
-        RIALTO_IPC_LOG_WARN("socket closed before event could be sent");
-        return false;
-    }
-    else if (TEMP_FAILURE_RETRY(sendmsg(it->second.sock, header, MSG_NOSIGNAL)) != static_cast<ssize_t>(requiredDataLen))
     {
-        RIALTO_IPC_LOG_SYS_ERROR(errno, "failed to send the complete event message");
-        return false;
-    }
+        std::unique_lock<std::mutex> locker(m_clientsLock);
 
-    locker.unlock();
+        auto it = m_clients.find(clientId);
+        if (it == m_clients.end() || it->second.sock < 0)
+        {
+            RIALTO_IPC_LOG_WARN("socket closed before event could be sent");
+            return false;
+        }
+        else if (TEMP_FAILURE_RETRY(sendmsg(it->second.sock, header, MSG_NOSIGNAL)) !=
+                 static_cast<ssize_t>(requiredDataLen))
+        {
+            RIALTO_IPC_LOG_SYS_ERROR(errno, "failed to send the complete event message");
+            return false;
+        }
+    }
 
     RIALTO_IPC_LOG_DEBUG("event{ %s } - { %s }", eventMessage->GetTypeName().c_str(),
                          eventMessage->ShortDebugString().c_str());

media/client/main/source/MediaPipeline.cpp

@@ -184,7 +184,7 @@ MediaPipeline::MediaPipeline(std::weak_ptr<IMediaPipelineClient> client, const V
                              const std::shared_ptr<common::IMediaFrameWriterFactory> &mediaFrameWriterFactory,
                              IClientController &clientController)
     : m_mediaPipelineClient(client), m_clientController{clientController}, m_currentAppState{ApplicationState::UNKNOWN},
-      m_mediaFrameWriterFactory(mediaFrameWriterFactory), m_currentState(State::IDLE)
+      m_mediaFrameWriterFactory(mediaFrameWriterFactory), m_currentState(State::IDLE), m_attachingSource(false)
 {
     RIALTO_CLIENT_LOG_DEBUG("entry:");
 
@@ -545,28 +545,29 @@ bool MediaPipeline::flush(int32_t sourceId, bool resetTime, bool &async)
 {
     RIALTO_CLIENT_LOG_DEBUG("entry:");
 
-    std::unique_lock<std::mutex> flushLock{m_flushMutex};
-    if (m_mediaPipelineIpc->flush(sourceId, resetTime, async))
     {
+        std::unique_lock<std::mutex> flushLock{m_flushMutex};
+        if (!m_mediaPipelineIpc->flush(sourceId, resetTime, async))
+        {
+            return false;
+        }
         m_attachedSources.setFlushing(sourceId, true);
-        flushLock.unlock();
+    }
 
-        // Clear all need datas for flushed source
-        std::lock_guard<std::mutex> lock{m_needDataRequestMapMutex};
-        for (auto it = m_needDataRequestMap.begin(); it != m_needDataRequestMap.end();)
+    // Clear all need datas for flushed source
+    std::lock_guard<std::mutex> lock{m_needDataRequestMapMutex};
+    for (auto it = m_needDataRequestMap.begin(); it != m_needDataRequestMap.end();)
+    {
+        if (it->second->sourceId == sourceId)
         {
-            if (it->second->sourceId == sourceId)
-            {
-                it = m_needDataRequestMap.erase(it);
-            }
-            else
-            {
-                ++it;
-            }
+            it = m_needDataRequestMap.erase(it);
+        }
+        else
+        {
+            ++it;
         }
-        return true;
     }
-    return false;
+    return true;
 }
 
 bool MediaPipeline::setSourcePosition(int32_t sourceId, int64_t position, bool resetTime, double appliedRate,

media/server/gstplayer/source/GstDecryptor.cpp

@@ -233,7 +233,7 @@ GstRialtoDecryptorPrivate::GstRialtoDecryptorPrivate(
     GstBaseTransform *parentElement,
     const std::shared_ptr<firebolt::rialto::wrappers::IGstWrapperFactory> &gstWrapperFactory,
     const std::shared_ptr<firebolt::rialto::wrappers::IGlibWrapperFactory> &glibWrapperFactory)
-    : m_decryptorElement(parentElement)
+    : m_decryptorElement(parentElement), m_decryptionService(nullptr)
 {
     if ((!gstWrapperFactory) || (!(m_gstWrapper = gstWrapperFactory->getGstWrapper())))
     {

media/server/gstplayer/source/GstGenericPlayer.cpp

@@ -223,9 +223,31 @@ GstGenericPlayer::~GstGenericPlayer()
     RIALTO_SERVER_LOG_DEBUG("GstGenericPlayer is destructed.");
     m_gstDispatcherThread.reset();
 
-    resetWorkerThread();
+    try
+    {
+        resetWorkerThread();
+    }
+    catch (const std::exception &e)
+    {
+        RIALTO_SERVER_LOG_ERROR("Exception during resetWorkerThread in destructor: %s", e.what());
+    }
+    catch (...)
+    {
+        RIALTO_SERVER_LOG_ERROR("Unknown exception during resetWorkerThread in destructor");
+    }
 
-    termPipeline();
+    try
+    {
+        termPipeline();
+    }
+    catch (const std::exception &e)
+    {
+        RIALTO_SERVER_LOG_ERROR("Exception during termPipeline in destructor: %s", e.what());
+    }
+    catch (...)
+    {
+        RIALTO_SERVER_LOG_ERROR("Unknown exception during termPipeline in destructor");
+    }
 }
 
 void GstGenericPlayer::initMsePipeline()

media/server/gstplayer/source/GstSrc.cpp

@@ -48,7 +48,9 @@ static void gstRialtoSrcFinalize(GObject *object)
 {
     GstRialtoSrc *src = GST_RIALTO_SRC(object);
     GstRialtoSrcPrivate *priv = src->priv;
+    GST_OBJECT_LOCK(src);
     g_free(priv->uri);
+    GST_OBJECT_UNLOCK(src);
     priv->~GstRialtoSrcPrivate();
     GST_CALL_PARENT(G_OBJECT_CLASS, finalize, (object));
 }

media/server/main/source/ActiveRequests.cpp

@@ -116,6 +116,7 @@ AddSegmentStatus ActiveRequests::addSegment(std::uint32_t requestId,
         return AddSegmentStatus::ERROR;
     }
 
+    std::unique_lock<std::mutex> lock{m_mutex};
     auto requestIter{m_requestMap.find(requestId)};
     if (requestIter != m_requestMap.end())
     {
@@ -127,6 +128,7 @@ AddSegmentStatus ActiveRequests::addSegment(std::uint32_t requestId,
 
 const IMediaPipeline::MediaSegmentVector &ActiveRequests::getSegments(std::uint32_t requestId) const
 {
+    std::unique_lock<std::mutex> lock{m_mutex};
     auto requestIter{m_requestMap.find(requestId)};
     if (requestIter != m_requestMap.end())
     {

media/server/main/source/NeedMediaData.cpp

@@ -31,7 +31,7 @@ NeedMediaData::NeedMediaData(std::weak_ptr<IMediaPipelineClient> client, IActive
                              const ISharedMemoryBuffer &shmBuffer, int sessionId, MediaSourceType mediaSourceType,
                              std::int32_t sourceId, PlaybackState currentPlaybackState)
     : m_client{client}, m_activeRequests{activeRequests}, m_mediaSourceType{mediaSourceType}, m_frameCount{kMaxFrames},
-      m_sourceId{sourceId}
+      m_sourceId{sourceId}, m_maxMediaBytes{0}
 {
     if (PlaybackState::PLAYING != currentPlaybackState)
     {