Hi Anders,
thanks for pointing out, I missed the term_fd_. I'll send out a new version.
/Thanks HansN
On 10/14/2016 10:34 AM, Anders Widell wrote:
> Hi!
>
> This code has degraded in one aspect: it does not return so quickly
> (or at all) when term_fd_ has been signalled. Could you try to fix
> this in some way? Other comments marked [AndersW] below.
>
> regards,
>
> Anders Widell
>
>
> On 10/11/2016 04:57 PM, Hans Nordeback wrote:
>> osaf/services/infrastructure/dtms/transport/tests/Makefile.am | 3 +-
>> osaf/services/infrastructure/dtms/transport/tests/transport_monitor_test.cc
>> | 4 +-
>> osaf/services/infrastructure/dtms/transport/transport_monitor.cc |
>> 41 +++++----
>> 3 files changed, 27 insertions(+), 21 deletions(-)
>>
>>
>> diff --git
>> a/osaf/services/infrastructure/dtms/transport/tests/Makefile.am
>> b/osaf/services/infrastructure/dtms/transport/tests/Makefile.am
>> --- a/osaf/services/infrastructure/dtms/transport/tests/Makefile.am
>> +++ b/osaf/services/infrastructure/dtms/transport/tests/Makefile.am
>> @@ -42,4 +42,5 @@ transport_test_SOURCES = \
>> transport_test_LDADD = \
>> $(GTEST_DIR)/lib/libgtest.la \
>> - $(GTEST_DIR)/lib/libgtest_main.la
>> + $(GTEST_DIR)/lib/libgtest_main.la \
>> + $(top_builddir)/osaf/libs/core/libopensaf_core.la
>> diff --git
>> a/osaf/services/infrastructure/dtms/transport/tests/transport_monitor_test.cc
>>
>> b/osaf/services/infrastructure/dtms/transport/tests/transport_monitor_test.cc
>>
>>
>> ---
>> a/osaf/services/infrastructure/dtms/transport/tests/transport_monitor_test.cc
>> +++
>> b/osaf/services/infrastructure/dtms/transport/tests/transport_monitor_test.cc
>> @@ -82,14 +82,14 @@ class TransportMonitorTest : public ::te
>> TEST_F(TransportMonitorTest, WaitForNonexistentDaemonName) {
>> pid_t pid = monitor_->WaitForDaemon("name_does_not_exist", 17);
>> EXPECT_EQ(pid, pid_t{-1});
>> - EXPECT_EQ(mock_osaf_poll.invocations, 17);
>> + EXPECT_EQ(mock_osaf_poll.invocations, 1);
>> }
>> TEST_F(TransportMonitorTest, WaitForNonexistentDaemonPid) {
>> CreatePidFile("pid_does_not_exist", 1234567890, false);
>> pid_t pid = monitor_->WaitForDaemon("pid_does_not_exist", 1);
>> EXPECT_EQ(pid, pid_t{-1});
>> - EXPECT_EQ(mock_osaf_poll.invocations, 1);
>> + EXPECT_EQ(mock_osaf_poll.invocations, 0);
>> }
>> TEST_F(TransportMonitorTest, WaitForExistingPid) {
>> diff --git
>> a/osaf/services/infrastructure/dtms/transport/transport_monitor.cc
>> b/osaf/services/infrastructure/dtms/transport/transport_monitor.cc
>> --- a/osaf/services/infrastructure/dtms/transport/transport_monitor.cc
>> +++ b/osaf/services/infrastructure/dtms/transport/transport_monitor.cc
>> @@ -25,15 +25,16 @@
>> #include "./configmake.h"
>> #include "osaf/libs/core/common/include/osaf_poll.h"
>> #include "osaf/libs/core/cplusplus/base/getenv.h"
>> +#include "osaf/libs/core/cplusplus/base/file_notify.h"
>> TransportMonitor::TransportMonitor(int term_fd)
>> : term_fd_{term_fd},
>> - pkgpiddir_{base::GetEnv<std::string>("pkgpiddir", PKGPIDDIR)},
>> - proc_path_{"/proc/"},
>> - mds_log_file_{base::GetEnv<std::string>("pkglogdir", PKGLOGDIR)
>> - + "/mds.log"},
>> - old_mds_log_file_{mds_log_file_ + ".old"},
>> - use_tipc_{base::GetEnv<std::string>("MDS_TRANSPORT", "TCP") ==
>> "TIPC"} {
>> +pkgpiddir_{base::GetEnv<std::string>("pkgpiddir", PKGPIDDIR)},
>> +proc_path_{"/proc/"},
>> +mds_log_file_{base::GetEnv<std::string>("pkglogdir", PKGLOGDIR)
>> + + "/mds.log"},
>> +old_mds_log_file_{mds_log_file_ + ".old"},
>> +use_tipc_{base::GetEnv<std::string>("MDS_TRANSPORT", "TCP") ==
>> "TIPC"} {
> [AndersW] Indentation has been messed up here.
>> }
>> TransportMonitor::~TransportMonitor() {
>> @@ -43,15 +44,17 @@ pid_t TransportMonitor::WaitForDaemon(co
>> int64_t seconds_to_wait) {
>> std::string pidfile = pkgpiddir_ + "/" + daemon_name + ".pid";
>> pid_t pid = pid_t{-1};
>> - for (int64_t count = 0; count != seconds_to_wait; ++count) {
>> - if (!(std::ifstream{pidfile} >> pid).fail()
>> - && IsDir(proc_path_ + std::to_string(pid)))
>> - break;
>> - pid = pid_t{-1};
>> - if (Sleep(1))
>> - return pid_t{-1};
>> + base::FileNotify file_notify;
>> + int rc = file_notify.WaitForFileCreation(pidfile, seconds_to_wait
>> * 1000);
> [AndersW] Possibly use kMillisPerSec from osaf_time.h instead of
> writing 1000 explicitly.
>> +
>> + if (rc == base::FileNotify::kOK) {
>> + if (!(std::ifstream{pidfile} >> pid).fail()) {
>> + if (IsDir(proc_path_ + std::to_string(pid))) {
>> + return pid;
>> + }
>> + }
>> }
>> - return pid;
>> + return pid_t{-1};
>> }
>> bool TransportMonitor::Sleep(int64_t seconds_to_wait) {
>> @@ -65,6 +68,7 @@ bool TransportMonitor::IsDir(const std::
>> }
>> void TransportMonitor::RotateMdsLogs(pid_t pid_to_watch) {
>> + base::FileNotify file_notify;
>> std::string pid_path{proc_path_ + std::to_string(pid_to_watch)};
>> for (;;) {
>> if (FileSize(mds_log_file_) > kMaxFileSize) {
>> @@ -72,10 +76,11 @@ void TransportMonitor::RotateMdsLogs(pid
>> rename(mds_log_file(), old_mds_log_file());
>> }
>> if (pid_to_watch != pid_t{0}) {
>> - for (int64_t i = 0; i != kLogRotationIntervalInSeconds; ++i) {
>> - if (!IsDir(pid_path) || Sleep(1))
>> - return;
>> - }
>> + int rc =
>> + file_notify.WaitForFileDeletion(pid_path,
>> + kLogRotationIntervalInSeconds * 1000);
>> + if (rc == base::FileNotify::kOK)
>> + return;
>> } else {
>> if (Sleep(kLogRotationIntervalInSeconds))
>> return;
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
