Hi Dat,
See my comment inline.
Best Regards,
Thien
________________________________
From: Dat Tran Quoc Phan <dat.tq.p...@dektech.com.au>
Sent: Monday, April 15, 2024 9:38 AM
To: Thang Duc Nguyen <thang.d.ngu...@dektech.com.au>; Thien Minh Huynh
<thien.m.hu...@dektech.com.au>; Tai Huynh Nguyen <tai.h.ngu...@dektech.com.au>
Cc: opensaf-devel@lists.sourceforge.net <opensaf-devel@lists.sourceforge.net>;
Dat Tran Quoc Phan <dat.tq.p...@dektech.com.au>
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, 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..a6ca87d75 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) {
+ return (!isLoggerBufferEmpty() &&
+ (queuedNotificationList.front()->getNotificationId() == invocation));
[Thien]: I suggest break down condition to easy read
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
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
