RDKECOREMW-1166: Exception Handling by Nairisnotavailable · Pull Request #353 · rdkcentral/entservices-inputoutput

Nairisnotavailable · 2025-12-31T16:11:21Z

Reason for change : Exception Handling
Test Procedure : Build is successful
Priority : Unprioritized
Risks : None

Reason for change : Exception Handling Test Procedure : Build is successful Priority : Unprioritized Risks : None

Removed printHeader calls from various process methods to streamline logging.

Removed printHeader calls from multiple process methods for cleaner logging.

rebase

Copilot

Pull request overview

This pull request improves exception handling in the HdmiCecSource and HdmiCecSink Thunder plugins to prevent crashes and ensure proper error propagation during initialization and runtime operations.

Key changes include:

Enhanced error handling in plugin Initialize() methods to validate Configure() results and clean up resources on failure
Addition of try-catch blocks around critical operations (CECEnable/CECDisable, device initialization, thread creation)
Improved Deinitialize() methods with better conditional checks for null pointers and connection state
Better exception propagation with explicit throw statements to maintain error context

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
HdmiCecSource/HdmiCecSource.cpp	Added Configure() error checking in Initialize(), restructured error path cleanup, improved Deinitialize() null safety, fixed spelling in Information()
HdmiCecSource/HdmiCecSourceImplementation.cpp	Added try-catch blocks around CECEnable/CECDisable calls, added exception handling for address acquisition and connection allocation, added throw statements for exception propagation
HdmiCecSink/HdmiCecSink.cpp	Added Configure() error checking in Initialize(), restructured Deinitialize() with better null checks, removed redundant profile checks, fixed spelling in Information()
HdmiCecSink/HdmiCecSinkImplementation.cpp	Added exception handling for device initialization and settings loading, moved thread initialization after core setup, added comprehensive cleanup in exception handlers, improved error logging levels
.github/workflows/L1-tests.yml	Added ccec/Exception.hpp to mock headers list for L1 testing

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.

rebase

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.

Copilot · 2026-01-12T14:44:22Z

+				return "Invalid service";
+			}

           ASSERT(nullptr != service);


The Initialize method checks for nullptr service at line 67-70, but then immediately follows with ASSERT(nullptr != service) at line 72. If the nullptr check is meant to handle null service gracefully in production builds, the ASSERT should be removed or placed before the nullptr check. In debug builds, the nullptr check at line 67 will never execute because the ASSERT would trigger first in the original flow. This creates inconsistent behavior between debug and release builds.

Suggested change

ASSERT(nullptr != service);

Copilot · 2026-01-12T14:44:22Z

+				LOGERR("CEC exception caught while getting addresses %s", e.what());
+				throw;
+			}
+			try {
+				smConnection = new Connection(logicalAddress.toInt(),false,"ServiceManager::Connection::");
+			} catch (const std::bad_alloc& e) {
+				LOGERR("smConnection allocation failed %s", e.what());


The exception handling in CECEnable() logs errors and re-throws exceptions, but these exceptions propagate to callers that return Core::ERROR_GENERAL without cleaning up resources that were successfully initialized (like LibCCEC initialization, thread creation). This can leave the plugin in an inconsistent state. Consider adding proper cleanup logic before rethrowing, or implementing a rollback mechanism that undoes partially completed initialization steps.

Suggested change

LOGERR("CEC exception caught while getting addresses %s", e.what());

throw;

}

try {

smConnection = new Connection(logicalAddress.toInt(),false,"ServiceManager::Connection::");

} catch (const std::bad_alloc& e) {

LOGERR("smConnection allocation failed %s", e.what());

LOGERR("CEC exception caught while getting addresses %s", e.what());

// Cleanup resources initialized earlier in CECEnable()

try {

if (m_sendKeyEventThread.get().joinable()) {

m_sendKeyEventThread.get().join();

}

} catch (const std::system_error& joinErr) {

LOGERR("exception while joining m_sendKeyEventThread during cleanup %s", joinErr.what());

}

throw;

}

try {

smConnection = new Connection(logicalAddress.toInt(),false,"ServiceManager::Connection::");

} catch (const std::bad_alloc& e) {

LOGERR("smConnection allocation failed %s", e.what());

// Cleanup resources initialized earlier in CECEnable()

try {

if (m_sendKeyEventThread.get().joinable()) {

m_sendKeyEventThread.get().join();

}

} catch (const std::system_error& joinErr) {

LOGERR("exception while joining m_sendKeyEventThread during cleanup %s", joinErr.what());

}

Copilot · 2026-01-12T14:44:22Z

           _service->AddRef();
-           _service->Register(&_notification);
           _hdmiCecSink = _service->Root<Exchange::IHdmiCecSink>(_connectionId, 5000, _T("HdmiCecSinkImplementation"));

           if(nullptr != _hdmiCecSink)
            {
-                _hdmiCecSink->Configure(service);
-                _hdmiCecSink->Register(&_notification);
-                Exchange::JHdmiCecSink::Register(*this, _hdmiCecSink);
-                LOGINFO("HdmiCecSink plugin is available. Successfully activated HdmiCecSink Plugin");
-            }
-            else
-            {
-                msg = "HdmiCecSink plugin is not available";
-                LOGINFO("HdmiCecSink plugin is not available. Failed to activate HdmiCecSink Plugin");
-            }
-
-            if (0 != msg.length())
-            {
-                Deinitialize(service);
-            }
-
-           // On success return empty, to indicate there is no error text.
-           return msg;
-        }
+				Core::hresult res = _hdmiCecSink->Configure(service);
+				if (res != Core::ERROR_NONE) {
+					msg = "HdmiCecSink plugin platform configuration error";
+					LOGERR("HdmiCecSink plugin platform configuration error. Failed to activate HdmiCecSink Plugin");
+					if (_connectionId != 0 && _service != nullptr) {
+						RPC::IRemoteConnection* connection = _service->RemoteConnection(_connectionId);
+						if (connection != nullptr) {
+							try {
+								connection->Terminate();
+							}
+							catch (const std::exception& e) {
+								LOGWARN("Failed to terminate connection: %s", e.what());
+							}
+							connection->Release();
+						}
+					}						
+					_hdmiCecSink->Release();
+					_hdmiCecSink = nullptr;
+					if (_service) {
+						PluginHost::IShell* tmp = _service;
+						tmp->Release(); 
+						_service = nullptr;
+					}
+					return msg;
+				}
+				LOGINFO("HdmiCecSink plugin platform configured successfully");
+				_service->Register(&_notification);


The _service->Register(&_notification) call has been moved to after Configure() succeeds. However, according to Thunder plugin lifecycle guidelines, the framework service registration should happen early in Initialize() to receive plugin state change notifications. Moving it after Configure() means the plugin won't receive state change events if Configure() is executing.

Copilot · 2026-01-12T14:44:23Z

+				   m_sendKeyEventThreadExit = false;
+				   m_sendKeyEventThread = std::thread(threadSendKeyEvent);
+				   m_currentArcRoutingState = ARC_STATE_ARC_TERMINATED;
+				   m_arcRoutingThread = std::thread(threadArcRouting);
+				   m_semSignaltoArcRoutingThread.acquire();				  


The thread initialization (m_sendKeyEventThread and m_arcRoutingThread) has been moved inside the try-catch block and is now done after CECEnable(). However, m_semSignaltoArcRoutingThread.acquire() at line 794 is called AFTER starting m_arcRoutingThread, which could cause a race condition. The semaphore should be acquired before or the thread should be started in a paused state.

Copilot · 2026-01-12T14:44:23Z

+			   return "Invalid service";
+			}

           ASSERT(nullptr != service);


The Initialize method checks for nullptr service at line 66-69, but then immediately follows with ASSERT(nullptr != service) at line 71. If the nullptr check is meant to handle null service gracefully in production builds, the ASSERT should be removed or placed before the nullptr check. This creates inconsistent behavior between debug and release builds.

Suggested change

ASSERT(nullptr != service);

Copilot · 2026-01-12T14:44:23Z

+				_connectionId = 0;
+				if (_service) {
+					PluginHost::IShell* tmp = _service;
+					tmp->Release();
+					_service = nullptr;
+				}
+			}


The cleanup logic manually releases _service using a temporary pointer pattern, but _connectionId is reset in the cleanup block after checking for errors. According to Thunder plugin lifecycle guidelines, all member variables should be reset to their initial state during cleanup. The _connectionId should be reset to 0 only after all cleanup is complete, and the pattern should match what's done in Deinitialize().

Copilot · 2026-01-12T14:44:24Z

+			LOGERR("IARM bus is not available. Failed to activate HdmiCecSource Plugin");
+			return Core::ERROR_GENERAL;
+		}
        ASSERT(_powerManagerPlugin);


The ASSERT at line 430 after the error handling blocks will never be executed if an error occurs (since the function returns Core::ERROR_GENERAL before reaching this point). However, if initialization succeeds without exceptions but _powerManagerPlugin is still nullptr, this ASSERT will trigger in a non-debug build without proper error handling. Consider adding an explicit null check with appropriate error handling instead of relying on ASSERT.

Copilot · 2026-01-12T14:44:24Z

+				if (_connectionId != 0 && _service != nullptr) {
+					RPC::IRemoteConnection* connection = _service->RemoteConnection(_connectionId);
+					if (connection != nullptr) {
+						try {
+							connection->Terminate();
+						}
+						catch (const std::exception& e) {
+							std::string errorMessage = "Failed to terminate connection: ";
+							errorMessage += e.what();
+							LOGWARN("%s", errorMessage.c_str());
+						}
+						connection->Release();
+					}
+				}					
            }
-
-            if (0 != msg.length())
-            {
-                Deinitialize(service);
-            }
-
-           // On success return empty, to indicate there is no error text.
-           return msg;
-        }
+
+			if (0 != msg.length())
+			{
+				if (_connectionId != 0 && _service != nullptr) {				
+					RPC::IRemoteConnection* connection = _service->RemoteConnection(_connectionId);
+					if (connection != nullptr) {
+						try {
+							connection->Terminate();
+						}
+						catch (const std::exception& e) {
+							std::string errorMessage = "Failed to terminate connection: ";
+							errorMessage += e.what();
+							LOGWARN("%s", errorMessage.c_str());
+						}
+						connection->Release();
+					}
+				}					


The duplicate connection termination and cleanup logic in Initialize() at lines 103-116 and 121-134 should be refactored into a single helper function to reduce code duplication and improve maintainability. This pattern is repeated twice in the same method.

Copilot · 2026-01-12T14:44:24Z

           _hdmiCecSource = _service->Root<Exchange::IHdmiCecSource>(_connectionId, 5000, _T("HdmiCecSourceImplementation"));
-
-           if(nullptr != _hdmiCecSource)
-            {
-                _hdmiCecSource->Configure(service);
-                _hdmiCecSource->Register(&_notification);
-                Exchange::JHdmiCecSource::Register(*this, _hdmiCecSource);
-                LOGINFO("HdmiCecSource plugin is available. Successfully activated HdmiCecSource Plugin");
-            }
-            else
-            {
-                msg = "HdmiCecSource plugin is not available";
+			if(nullptr != _hdmiCecSource)
+			{
+				Core::hresult res = _hdmiCecSource->Configure(service);
+				if (res != Core::ERROR_NONE)
+				{
+					msg = "HdmiCecSource plugin platform configuration error";
+					LOGINFO("HdmiCecSource plugin configuration failed. Failed to activate HdmiCecSource Plugin");
+					_hdmiCecSource->Release();
+					_hdmiCecSource = nullptr;
+				}
+				else
+				{
+					_service->Register(&_notification);


The registration of _service and _hdmiCecSource with the notification listener has been moved from Initialize() start to after successful Configure(). However, if Configure() fails, the code attempts to clean up resources without ever having registered with _service. The _service->Register(&_notification) should be called before Configure() and unregistered in the cleanup path if Configure() fails, as per Thunder plugin lifecycle guidelines.

rebase

RDKECOREMW-1166: Exception Handling

0451b4d

Reason for change : Exception Handling Test Procedure : Build is successful Priority : Unprioritized Risks : None

Nairisnotavailable requested a review from a team as a code owner December 31, 2025 16:11

Nairisnotavailable added 6 commits December 31, 2025 22:11

Remove printHeader calls from process methods

ef34c4c

Removed printHeader calls from various process methods to streamline logging.

Add Exception.hpp to L1-tests workflow

e82708b

Remove printHeader calls in process methods

c051bd4

Removed printHeader calls from multiple process methods for cleaner logging.

Resolve build error

ece4734

Reorder unregistering and instance reset

b9002ad

Merge pull request #355 from rdkcentral/develop

0b77fd0

rebase

arun-madhavan-013 requested a review from Copilot January 7, 2026 15:19

Copilot started reviewing on behalf of arun-madhavan-013 January 7, 2026 15:19 View session

Copilot AI reviewed Jan 7, 2026

View reviewed changes

Indentation update HdmiCecSourceImplementation.cpp

94ad034

arun-madhavan-013 requested a review from Copilot January 7, 2026 16:39

Copilot started reviewing on behalf of arun-madhavan-013 January 7, 2026 16:39 View session

Copilot AI reviewed Jan 7, 2026

View reviewed changes

Nairisnotavailable added 7 commits January 7, 2026 22:35

Update HdmiCecSourceImplementation.cpp

7bd6953

Update HdmiCecSink.cpp

cbe9255

Update HdmiCecSinkImplementation.cpp

2c4873e

Update HdmiCecSinkImplementation.cpp

dcd9f45

Update HdmiCecSinkImplementation.cpp

b515aa1

Update HdmiCecSource.cpp

831b1b1

Update HdmiCecSourceImplementation.cpp

b496d0d

arun-madhavan-013 requested a review from Copilot January 7, 2026 18:40

Copilot started reviewing on behalf of arun-madhavan-013 January 7, 2026 18:41 View session

Copilot AI reviewed Jan 7, 2026

View reviewed changes

Nairisnotavailable added 4 commits January 8, 2026 00:23

Update HdmiCecSink.cpp

e8f93c8

Merge pull request #357 from rdkcentral/develop

e57f0a8

rebase

Update HdmiCecSink.cpp

fd916c8

Update HdmiCecSinkImplementation.cpp

bf7222b