Hi! I have identified a problem with this change: inotify doesn't work together with the /proc file system. The thing that happens when I kill osafdtmd is that it takes up to 15 seconds for inotify to time out, and then in the next iteration of the loop it will detect that the directory is already deleted (ENOENT). So it is OK to wait for the creation of the pid file, but the code that monitors the process until it dies needs to be reconsidered.
Maybe we can look at some of the suggestions presented here: http://stackoverflow.com/questions/1157700/how-to-wait-for-exit-of-non-children-processes / Anders Widell On 10/17/2016 06:25 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 > | 38 ++++++--- > 3 files changed, 30 insertions(+), 15 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, 0); > } > > 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 > @@ -23,8 +23,11 @@ > #include <cstdio> > #include <fstream> > #include "./configmake.h" > +#include "logtrace.h" > +#include "osaf_time.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}, > @@ -43,19 +46,23 @@ 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; > + base::FileNotify::FileNotifyErrors rc = > + file_notify.WaitForFileCreation(pidfile, seconds_to_wait * > kMillisPerSec, > + term_fd_); > + > + if (rc == base::FileNotify::FileNotifyErrors::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) { > - return osaf_poll_one_fd(term_fd_, seconds_to_wait * 1000) != 0; > + return osaf_poll_one_fd(term_fd_, seconds_to_wait * kMillisPerSec) != 0; > } > > bool TransportMonitor::IsDir(const std::string& path) { > @@ -65,6 +72,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,9 +80,15 @@ 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; > + base::FileNotify::FileNotifyErrors rc = > + file_notify.WaitForFileDeletion( > + pid_path, > + kLogRotationIntervalInSeconds * kMillisPerSec, > + term_fd_); > + if (rc != base::FileNotify::FileNotifyErrors::kTimeOut) { > + return; > + } else { > + TRACE("Timeout received, continuing..."); > } > } else { > if (Sleep(kLogRotationIntervalInSeconds)) ------------------------------------------------------------------------------ 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
