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
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
