Re: [devel] [PATCH 1/1] ntf: check existed notification before retrieving [#3350]

[Thien M. H. <thi...@de...>]

Hi Dat,

ACK from me.

Best Regards,
Thien

________________________________
From: Dat Tran Quoc Phan <dat...@de...>
Sent: Monday, April 15, 2024 5:10 PM
To: Thang Duc Nguyen <tha...@de...>; Thien Minh Huynh <thi...@de...>; Tai Huynh Nguyen <tai...@de...>
Cc: ope...@li... <ope...@li...>; Dat Tran Quoc Phan <dat...@de...>
Subject: [PATCH 1/1] ntf: check existed notification before retrieving [#3350]

Related to ticket #3349, when ntf removes an overdue notification
from logger buffer, it will try to retrieve next notification
in queue. In case there's only a single notification in queue
and it's overdue, after removing it, ntf will try to retrieve
non-existed notification, causing ntf crash.

Solution is to check if there's existed notification in queue
before retrieving it. Also, improve checking existed notification
when logd calls saLogWriteCallback.
---
 src/ntf/ntfd/NtfLogger.cc | 8 +++++---
 src/ntf/ntfd/NtfLogger.h  | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/ntf/ntfd/NtfLogger.cc b/src/ntf/ntfd/NtfLogger.cc
index 2b5c501ee..480c9e7b6 100644
--- a/src/ntf/ntfd/NtfLogger.cc
+++ b/src/ntf/ntfd/NtfLogger.cc
@@ -108,7 +108,7 @@ void saLogStreamOpenCallback(SaInvocationT invocation,

 void saLogWriteLogCallback(SaInvocationT invocation, SaAisErrorT error) {
   TRACE_ENTER2("Callback for notificationId %llu", invocation);
-  if (NtfAdmin::theNtfAdmin->logger.getFrontNotificationId() != invocation) {
+  if (!NtfAdmin::theNtfAdmin->logger.isExistNotification(invocation)) {
     TRACE("Notification had been processed by logd, but Id is not existed"
           " in logger. Probably due to notification overdue, ignore "
           "notificationId %llu", invocation);
@@ -384,8 +384,9 @@ void NtfLogger::disableAckWaiting() {
   notification->setWaitingAck(false);
 }

-SaNtfIdentifierT NtfLogger::getFrontNotificationId() {
-  return (queuedNotificationList.front()->getNotificationId());
+bool NtfLogger::isExistNotification(SaInvocationT invocation) {
+  if (isLoggerBufferEmpty()) return false;
+  return (queuedNotificationList.front()->getNotificationId() == invocation);
 }

 void NtfLogger::logQueuedNotification() {
@@ -397,6 +398,7 @@ void NtfLogger::logQueuedNotification() {
       dequeueNotification();
       resetLoggerBufferFullFlag();
       sendLoggedConfirm(notification->getNotificationId());
+      if (isLoggerBufferEmpty()) return;
       notification = queuedNotificationList.front();
     }
     if (notification->isWaitingAck()) return;
diff --git a/src/ntf/ntfd/NtfLogger.h b/src/ntf/ntfd/NtfLogger.h
index 283fd3fe2..4af8fc3d8 100644
--- a/src/ntf/ntfd/NtfLogger.h
+++ b/src/ntf/ntfd/NtfLogger.h
@@ -73,7 +73,7 @@ class NtfLogger {
   void logQueuedNotification();
   bool isLoggerBufferEmpty() { return queuedNotificationList.empty(); }
   void disableAckWaiting();
-  SaNtfIdentifierT getFrontNotificationId();
+  bool isExistNotification(SaInvocationT invocation);

  private:
   SaAisErrorT initLog();
--
2.25.1