Changeset 98071 in vbox for trunk/src/VBox/Main/src-server

Timestamp:

Jan 13, 2023 10:38:26 AM (2 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

155196

Message:

Main/HostDnsServiceLinux.cpp: Massaged the resolv.conf monitoring code a bit more. bugref:10255

Location:

trunk/src/VBox/Main/src-server

Files:

• HostDnsService.h (modified) (2 diffs)
• linux/HostDnsServiceLinux.cpp (modified) (17 diffs)

trunk/src/VBox/Main/src-server/HostDnsService.h

@@ -249 +249 @@
 public:
 
-    HostDnsServiceLinux() : HostDnsServiceResolvConf(true) {}
+    HostDnsServiceLinux() : HostDnsServiceResolvConf(true), m_fdShutdown(-1) {}
     virtual ~HostDnsServiceLinux();
 
@@ -260 +260 @@
     int monitorThreadShutdown(RTMSINTERVAL uTimeoutMs);
     int monitorThreadProc(void);
+
+    /** Socket end to write shutdown notification to, so the monitor thread will
+     *  wake up and terminate. */
+    int m_fdShutdown;
 };
 

trunk/src/VBox/Main/src-server/linux/HostDnsServiceLinux.cpp

@@ -66 +66 @@
 *   Global Variables                                                                                                             *
 *********************************************************************************************************************************/
-static int g_DnsMonitorStop[2];
-
 static const char g_szEtcFolder[]          = "/etc";
 static const char g_szResolvConfPath[]     = "/etc/resolv.conf";
@@ -73 +71 @@
 
 
-class FileDescriptor
-{
-public:
-    FileDescriptor(int d = -1)
-        : fd(d)
-    {}
-
-    virtual ~FileDescriptor() {
-        if (fd != -1)
-            close(fd);
+HostDnsServiceLinux::~HostDnsServiceLinux()
+{
+    if (m_fdShutdown >= 0)
+    {
+        close(m_fdShutdown);
+        m_fdShutdown = -1;
     }
-
-    int fileDescriptor() const {return fd;}
-
-protected:
-    int fd;
-};
-
-
-class AutoNotify : public FileDescriptor
-{
-public:
-    AutoNotify()
-    {
-        FileDescriptor::fd = inotify_init();
-        AssertReturnVoid(FileDescriptor::fd != -1);
-    }
-};
-
-HostDnsServiceLinux::~HostDnsServiceLinux()
-{
 }
 
@@ -115 +89 @@
     RT_NOREF(uTimeoutMs);
 
-    send(g_DnsMonitorStop[0], "", 1, 0);
-
-    /** @todo r=andy Do we have to wait for something here? Can this fail? */
+    if (m_fdShutdown >= 0)
+        send(m_fdShutdown, "", 1, MSG_NOSIGNAL);
+
     return VINF_SUCCESS;
 }
 
-#ifdef LOG_ENABLED
 /**
  * Format the notifcation event mask into a buffer for logging purposes.
@@ -170 +143 @@
     return psz;
 }
-#endif
-
 
 /**
@@ -204 +175 @@
 }
 
+/** @todo If this code is needed elsewhere, we should abstract it into an IPRT
+ *        thingy that monitors a file (path) for changes.  This code is a little
+ *        bit too complex to be duplicated. */
 int HostDnsServiceLinux::monitorThreadProc(void)
 {
     /*
-     * inotify initialization.
-     *
-     * The order here helps keep the descriptor values stable.
+     * Create a socket pair for signalling shutdown (see monitorThreadShutdown).
+     * ASSUME Linux 2.6.27 or later and that we can use SOCK_CLOEXEC.
+     */
+    int aiStopPair[2];
+    int rc = socketpair(AF_LOCAL, SOCK_DGRAM | SOCK_CLOEXEC, 0, aiStopPair);
+    int iErr = errno;
+    AssertLogRelMsgReturn(rc == 0, ("socketpair: failed (%d: %s)\n", iErr, strerror(iErr)), RTErrConvertFromErrno(iErr));
+
+    m_fdShutdown = aiStopPair[0];
+
+    onMonitorThreadInitDone();
+
+    /*
+     * inotify initialization (using inotify_init1 w/ IN_CLOEXEC introduced
+     * in 2.6.27 shouldn't be a problem any more).
      *
      * Note! Ignoring failures here is safe, because poll will ignore entires
      *       with negative fd values.
      */
-    AutoNotify Notify;
-
-    /* Monitor the /etc directory so we can detect moves, unliking and creations
+    int const iNotifyFd = inotify_init1(IN_CLOEXEC);
+    if (iNotifyFd < 0)
+        LogRel(("HostDnsServiceLinux::monitorThreadProc: Warning! inotify_init failed (errno=%d)\n", errno));
+
+    /* Monitor the /etc directory so we can detect moves, creating and unlinking
        involving /etc/resolv.conf:  */
-    int const iWdDir = inotify_add_watch(Notify.fileDescriptor(), g_szEtcFolder, IN_MOVE | IN_CREATE | IN_DELETE);
+    int const iWdDir = inotify_add_watch(iNotifyFd, g_szEtcFolder, IN_MOVE | IN_CREATE | IN_DELETE);
 
     /* In case g_szResolvConfPath is a symbolic link, monitor the target directory
-       too for changes to what it links to. */
+       too for changes to what it links to (kept up to date via iWdDir). */
     char   szRealResolvConf[PATH_MAX];
     size_t offRealResolvConfName = 0;
-    int iWdSymDir = ::monitorSymlinkedDir(Notify.fileDescriptor(), szRealResolvConf, &offRealResolvConfName);
+    int iWdSymDir = ::monitorSymlinkedDir(iNotifyFd, szRealResolvConf, &offRealResolvConfName);
 
     /* Monitor the resolv.conf itself if it exists, following all symlinks. */
-    int iWdFile = inotify_add_watch(Notify.fileDescriptor(), g_szResolvConfPath, IN_CLOSE_WRITE | IN_DELETE_SELF);
-
-    Log5Func(("iWdDir=%d iWdSymDir=%d iWdFile=%d\n", iWdDir, iWdSymDir, iWdFile));
-
-    /*
-     * Create a socket pair for signalling shutdown via (see monitorThreadShutdown).
-     */
-    int rc = socketpair(AF_LOCAL, SOCK_DGRAM, 0, g_DnsMonitorStop);
-    AssertMsgReturn(rc == 0, ("socketpair: failed (%d: %s)\n", errno, strerror(errno)), E_FAIL);
-
-    /* automatic cleanup tricks */
-    FileDescriptor stopper0(g_DnsMonitorStop[0]);
-    FileDescriptor stopper1(g_DnsMonitorStop[1]);
+    int iWdFile = inotify_add_watch(iNotifyFd, g_szResolvConfPath, IN_CLOSE_WRITE | IN_DELETE_SELF);
+
+    LogRel5(("HostDnsServiceLinux::monitorThreadProc: inotify: %d - iWdDir=%d iWdSymDir=%d iWdFile=%d\n",
+             iNotifyFd, iWdDir, iWdSymDir, iWdFile));
 
     /*
@@ -247 +226 @@
     RT_ZERO(aFdPolls);
 
-    aFdPolls[0].fd = Notify.fileDescriptor();
+    aFdPolls[0].fd = iNotifyFd;
     aFdPolls[0].events = POLLIN;
 
-    aFdPolls[1].fd = g_DnsMonitorStop[1];
+    aFdPolls[1].fd = aiStopPair[1];
     aFdPolls[1].events = POLLIN;
-
-    onMonitorThreadInitDone();
 
     /*
      * The monitoring loop.
      */
+    int vrcRet = VINF_SUCCESS;
     for (;;)
     {
@@ -266 +244 @@
         if (rc == -1)
         {
-            LogRelMax(32, ("HostDnsServiceLinux::monitorThreadProc: poll failed %d: errno=%d\n", rc, errno));
-            RTThreadSleep(1);
+            if (errno != EINTR)
+            {
+                LogRelMax(32, ("HostDnsServiceLinux::monitorThreadProc: poll failed %d: errno=%d\n", rc, errno));
+                RTThreadSleep(1);
+            }
             continue;
         }
         Log5Func(("poll returns %d: [0]=%#x [1]=%#x\n", rc, aFdPolls[1].revents, aFdPolls[0].revents));
 
-        AssertMsgReturn(   (aFdPolls[0].revents & (POLLERR | POLLNVAL)) == 0
-                        && (aFdPolls[1].revents & (POLLERR | POLLNVAL)) == 0, ("Debug Me"), VERR_INTERNAL_ERROR);
-
+        AssertMsgBreakStmt(   (aFdPolls[0].revents & (POLLERR | POLLNVAL)) == 0 /* (ok for fd=-1 too, revents=0 then) */
+                           && (aFdPolls[1].revents & (POLLERR | POLLNVAL)) == 0,
+                              ("Debug Me: [0]=%d,%#x [1]=%d, %#x\n",
+                               aFdPolls[0].fd, aFdPolls[0].revents, aFdPolls[0].fd, aFdPolls[1].revents),
+                           vrcRet = VERR_INTERNAL_ERROR);
 
         /*
@@ -280 +263 @@
          */
         if (aFdPolls[1].revents & POLLIN)
-            return VINF_SUCCESS;
+            break; /** @todo should probably drain aiStopPair[1] here if we're really paranoid.
+                    * we'll be closing our end of the socket/pipe, so any stuck write
+                    * should return too (ECONNRESET, ENOTCONN or EPIPE). */
 
         if (aFdPolls[0].revents & POLLIN)
@@ -294 +279 @@
             } uEvtBuf;
 
-            ssize_t cbEvents = read(Notify.fileDescriptor(), &uEvtBuf, sizeof(uEvtBuf));
+            ssize_t cbEvents = read(iNotifyFd, &uEvtBuf, sizeof(uEvtBuf));
             Log5Func(("read(inotify) -> %zd\n", cbEvents));
             if (cbEvents > 0)
@@ -313 +298 @@
             while (cbEvents >= (ssize_t)INOTIFY_EVENT_SIZE)
             {
-#ifdef LOG_ENABLED
                 char szTmp[64];
                 if (pCurEvt->len == 0)
-                    Log5Func(("event: wd=%#x mask=%#x (%s) cookie=%#x\n", pCurEvt->wd, pCurEvt->mask,
-                              InotifyMaskToStr(szTmp, sizeof(szTmp), pCurEvt->mask), pCurEvt->cookie));
+                    LogRel5(("HostDnsServiceLinux::monitorThreadProc: event: wd=%#x mask=%#x (%s) cookie=%#x\n",
+                             pCurEvt->wd, pCurEvt->mask, InotifyMaskToStr(szTmp, sizeof(szTmp), pCurEvt->mask), pCurEvt->cookie));
                 else
-                    Log5Func(("event: wd=%#x mask=%#x (%s) cookie=%#x len=%#x '%s'\n",
+                    LogRel5(("HostDnsServiceLinux::monitorThreadProc: event: wd=%#x mask=%#x (%s) cookie=%#x len=%#x '%s'\n",
                               pCurEvt->wd, pCurEvt->mask, InotifyMaskToStr(szTmp, sizeof(szTmp), pCurEvt->mask),
                               pCurEvt->cookie, pCurEvt->len, pCurEvt->name));
-#endif
 
                 /*
@@ -339 +322 @@
                         if (iWdFileNew != -1)
                         {
-                            rc = inotify_rm_watch(Notify.fileDescriptor(), iWdFileNew);
+                            rc = inotify_rm_watch(iNotifyFd, iWdFileNew);
                             AssertMsg(rc >= 0, ("%d/%d\n", rc, errno));
                             iWdFileNew = -1;
@@ -367 +350 @@
                             if (iWdFileNew >= 0)
                             {
-                                rc = inotify_rm_watch(Notify.fileDescriptor(), iWdFileNew);
+                                rc = inotify_rm_watch(iNotifyFd, iWdFileNew);
                                 Log5Func(("dir: moved / created / deleted: dropped file watch (%d - rc=%d/err=%d)\n",
                                           iWdFileNew, rc, errno));
@@ -374 +357 @@
                             if (iWdSymDirNew >= 0)
                             {
-                                rc = inotify_rm_watch(Notify.fileDescriptor(), iWdSymDirNew);
+                                rc = inotify_rm_watch(iNotifyFd, iWdSymDirNew);
                                 Log5Func(("dir: moved / created / deleted: dropped symlinked dir watch (%d - %s/%s - rc=%d/err=%d)\n",
                                           iWdSymDirNew, szRealResolvConf, &szRealResolvConf[offRealResolvConfName], rc, errno));
@@ -385 +368 @@
                                 fTryReRead = true;
 
-                                iWdSymDirNew = ::monitorSymlinkedDir(Notify.fileDescriptor(),
-                                                                     szRealResolvConf, &offRealResolvConfName);
+                                iWdSymDirNew = ::monitorSymlinkedDir(iNotifyFd, szRealResolvConf, &offRealResolvConfName);
                                 if (iWdSymDirNew < 0)
                                     Log5Func(("dir: moved_to / created: re-stablished symlinked-directory monitoring: iWdSymDir=%d (%s/%s)\n",
@@ -415 +397 @@
                         if (iWdFileNew >= 0)
                         {
-                            rc = inotify_rm_watch(Notify.fileDescriptor(), iWdFileNew);
+                            rc = inotify_rm_watch(iNotifyFd, iWdFileNew);
                             Log5Func(("symdir: moved / created / deleted: drop file watch (%d - rc=%d/err=%d)\n",
                                       iWdFileNew, rc, errno));
@@ -452 +434 @@
             if (iWdFile == -1)
             {
-                iWdFile = inotify_add_watch(Notify.fileDescriptor(), g_szResolvConfPath, IN_CLOSE_WRITE | IN_DELETE_SELF);
+                iWdFile = inotify_add_watch(iNotifyFd, g_szResolvConfPath, IN_CLOSE_WRITE | IN_DELETE_SELF);
                 if (iWdFile >= 0)
                 {
@@ -467 +449 @@
             {
                 Log5Func(("Calling readResolvConf()...\n"));
-                readResolvConf();
+                try
+                {
+                    readResolvConf();
+                }
+                catch (...)
+                {
+                    LogRel(("HostDnsServiceLinux::monitorThreadProc: readResolvConf threw exception!\n"));
+                }
             }
         }
     }
-}
-
+
+    /*
+     * Close file descriptors.
+     */
+    if (aiStopPair[0] == m_fdShutdown) /* paranoia */
+    {
+        m_fdShutdown = -1;
+        close(aiStopPair[0]);
+    }
+    close(aiStopPair[1]);
+    close(iNotifyFd);
+    return vrcRet;
+}
+