From dc5ceda9e498d9e81eb55472e35e14782e9c533d Mon Sep 17 00:00:00 2001
From: Gerardo Ganis <Gerardo.Ganis@cern.ch>
Date: Mon, 22 Nov 2010 10:44:47 +0000
Subject: [PATCH] Import some consolidation fixes in the bonjour module

git-svn-id: http://root.cern.ch/svn/root/trunk@36834 27541ba8-7e3a-0410-8455-c3a389f83636
---
 net/xrootd/src/xrootd/src/XrdOuc/GNUmakefile  |  2 +-
 .../xrootd/src/XrdOuc/XrdOucAppleBonjour.cc   | 48 +++++++-----
 .../xrootd/src/XrdOuc/XrdOucAppleBonjour.hh   |  2 +-
 .../xrootd/src/XrdOuc/XrdOucAvahiBonjour.cc   | 75 +++++++++++--------
 .../xrootd/src/XrdOuc/XrdOucAvahiBonjour.hh   |  2 +-
 .../src/xrootd/src/XrdOuc/XrdOucBonjour.cc    |  8 +-
 .../src/xrootd/src/XrdOuc/XrdOucBonjour.hh    |  9 ++-
 net/xrootd/src/xrootd/src/XrdVersion.hh       |  2 +-
 8 files changed, 91 insertions(+), 57 deletions(-)

diff --git a/net/xrootd/src/xrootd/src/XrdOuc/GNUmakefile b/net/xrootd/src/xrootd/src/XrdOuc/GNUmakefile
index ce3052a5160..968482fd770 100644
--- a/net/xrootd/src/xrootd/src/XrdOuc/GNUmakefile
+++ b/net/xrootd/src/xrootd/src/XrdOuc/GNUmakefile
@@ -190,7 +190,7 @@ $(OBJDIR)/XrdOucBonjour.o: XrdOucBonjour.cc XrdOucBonjour.hh XrdSysError.hh \
 
 $(OBJDIR)/XrdOucFactoryBonjour.o: XrdOucBonjour.hh XrdSysError.hh \
                                 XrdSysHeaders.hh XrdOucString.hh XrdSysPthread.hh \
-                                XrdOucFactoryBonjour.hh XrdConfig.hh \
+                                XrdOucFactoryBonjour.hh XrdOucFactoryBonjour.cc XrdConfig.hh \
                                 XrdInet.hh XrdProtLoad.hh
 	@echo Compiling XrdOucFactoryBonjour.cc
 	$(ECHO)$(CC) -c $(CFLAGS) $(CFHASLIBDNSSD) $(INCLUDE) XrdOucFactoryBonjour.cc -o $(OBJDIR)/XrdOucFactoryBonjour.o
diff --git a/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAppleBonjour.cc b/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAppleBonjour.cc
index 14a9ad8dfd0..0c8b01eb92b 100644
--- a/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAppleBonjour.cc
+++ b/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAppleBonjour.cc
@@ -217,9 +217,10 @@ void XrdOucAppleBonjour::BrowseReply(DNSServiceRef ref, DNSServiceFlags flags,
       // Start resolution of the name.
       instance->ResolveNodeInformation(node);
 
-      instance->LockNodeList();
-      instance->ListOfNodes.push_back(node);
-      instance->UnLockNodeList();
+      // We are going to wait to add the node until it is completely resolved.
+      //instance->LockNodeList();
+      //instance->ListOfNodes.push_back(node);
+      //instance->UnLockNodeList();
 
       XrdLog.Say("------ XrdOucBonjour: discovered a new node: ", name);
    } else {
@@ -231,11 +232,11 @@ void XrdOucAppleBonjour::BrowseReply(DNSServiceRef ref, DNSServiceFlags flags,
       instance->UnLockNodeList();
 
       XrdLog.Say("------ XrdOucBonjour: the node ", name, " went out the network");
-   }
 
-   // Notify updates if there wont be more updates in a short period of time.
-   if (!(flags & kDNSServiceFlagsMoreComing))
-      callbackID->callback(callbackID->context);
+      // Notify updates if there wont be more updates in a short period of time.
+      if (!(flags & kDNSServiceFlagsMoreComing))
+         callbackID->callback(callbackID->context);
+   }
 }
 
 /******************************************************************************/
@@ -266,24 +267,37 @@ void XrdOucAppleBonjour::ResolveReply(DNSServiceRef ref, DNSServiceFlags flags,
                                       uint16_t port, uint16_t txtLen,
                                       const unsigned char * txtVal, void * context)
 {
-   XrdOucBonjourNode * node;
+   XrdOucAppleBonjour * instance;
+   XrdOucBonjourResolutionEntry * nodeAndCallback;
 
    if (error != kDNSServiceErr_NoError) {
       XrdLog.Emsg("OucBonjour", error, "complete the resolve callback");
       return;
    }
 
-   node = static_cast<XrdOucBonjourNode *>(context);
+   nodeAndCallback = static_cast<XrdOucBonjourResolutionEntry *>(context);
 
    // Copy the information of resolution results to the node.
-   node->SetHostName(hostname);
-   node->SetPort(ntohs(port));
+   nodeAndCallback->node->SetHostName(hostname);
+   nodeAndCallback->node->SetPort(ntohs(port));
 
    // Also, copy the TXT values.
-   node->GetBonjourRecord().AddRawTXTRecord((const char *)txtVal);
+   nodeAndCallback->node->GetBonjourRecord().AddRawTXTRecord((const char *)txtVal);
+
+   // Get the context (the XrdOucBonjour object which holds the lists of nodes).
+   instance = &XrdOucAppleBonjour::getInstance();
+
+   // We are ready to add the node to the list and invoke the callback.
+   instance->LockNodeList();
+   instance->ListOfNodes.push_back(node);
+   instance->UnLockNodeList();
+
+   // Notify updates if there wont be more updates in a short period of time.
+   if (!(flags & kDNSServiceFlagsMoreComing))
+      callbackID->callback(callbackID->context);
 }
 
-int XrdOucAppleBonjour::ResolveNodeInformation(XrdOucBonjourNode * nodeInfo)
+int XrdOucAppleBonjour::ResolveNodeInformation(XrdOucBonjourResolutionEntry * nodeAndCallback)
 {
    DNSServiceErrorType err;
    DNSServiceRef serviceRef;
@@ -295,11 +309,11 @@ int XrdOucAppleBonjour::ResolveNodeInformation(XrdOucBonjourNode * nodeInfo)
    err = DNSServiceResolve(&serviceRef,
                            0,
                            0,
-                           nodeInfo->GetBonjourRecord().GetServiceName(),
-                           nodeInfo->GetBonjourRecord().GetRegisteredType(),
-                           nodeInfo->GetBonjourRecord().GetReplyDomain(),
+                           nodeAndCallback->node->GetBonjourRecord().GetServiceName(),
+                           nodeAndCallback->node->GetBonjourRecord().GetRegisteredType(),
+                           nodeAndCallback->node->GetBonjourRecord().GetReplyDomain(),
                            ResolveReply,
-                           nodeInfo);
+                           nodeAndCallback);
 
    // Check for errors
    if (err != kDNSServiceErr_NoError) {
diff --git a/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAppleBonjour.hh b/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAppleBonjour.hh
index 1c3d71c0f73..794a233f869 100644
--- a/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAppleBonjour.hh
+++ b/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAppleBonjour.hh
@@ -107,7 +107,7 @@ public:
    // and port. It is important to use the resolution by-demand since the list
    // may not contain updated information due to the use of highly dynamical
    // DHCP and APIPA addresses.
-   int ResolveNodeInformation(XrdOucBonjourNode * nodeInfo);
+   int ResolveNodeInformation(XrdOucBonjourResolutionEntry * nodeAndCallback);
 
    // Accessor to get the singleton instance.
    static XrdOucAppleBonjour &getInstance();
diff --git a/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAvahiBonjour.cc b/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAvahiBonjour.cc
index fb4b2e1f659..f7549e5369a 100644
--- a/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAvahiBonjour.cc
+++ b/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAvahiBonjour.cc
@@ -208,6 +208,7 @@ void XrdOucAvahiBonjour::BrowseReply(AvahiServiceBrowser *b,
    XrdOucBonjourSubscribedEntry * callbackID;
    XrdOucAvahiBonjour *instance;
    XrdOucAvahiBonjourSearchNode predicate(name);
+   XrdOucBonjourResolutionEntry * toResolve;
 
    callbackID = (XrdOucBonjourSubscribedEntry *)userdata;
 
@@ -221,22 +222,20 @@ void XrdOucAvahiBonjour::BrowseReply(AvahiServiceBrowser *b,
 
       case AVAHI_BROWSER_NEW:
 
-         // ADD a new node to the list.
-         callbackID->node = new XrdOucBonjourNode(name, type, domain);
-
-         // Start resolution of the name.
-         if (!(avahi_service_resolver_new(callbackID->client, interface, protocol, name, type, domain, AVAHI_PROTO_INET, (AvahiLookupFlags)0, ResolveReply, callbackID)))
-            XrdLog.Emsg("OucBonjourRESOLVER", avahi_strerror(avahi_client_errno(callbackID->client)));
+        instance->LockNodeList();
+        // ADD a new node to the list.
+        toResolve = (XrdOucBonjourResolutionEntry *)malloc(sizeof(XrdOucBonjourResolutionEntry));
+        toResolve->node = new XrdOucBonjourNode(name, type, domain);
+        toResolve->callbackID = callbackID;
 
          XrdLog.Say("------ XrdOucBonjour: discovered a new node: ", name);
 
-         instance->LockNodeList();
-         instance->ListOfNodes.push_back(callbackID->node);
-         instance->UnLockNodeList();
+         // Start resolution of the name.
+         if (!(avahi_service_resolver_new(callbackID->client, interface, protocol, name, type, domain, AVAHI_PROTO_INET, (AvahiLookupFlags)0, ResolveReply, toResolve)))
+            XrdLog.Emsg("OucBonjour", avahi_strerror(avahi_client_errno(callbackID->client)));
 
-         // We must wait to invoke the callback to the node be resolved his
-         // name and port, at least.
-         //callbackID->callback(callbackID->context);
+        // Wait until the node is rsolved to insert it on the list AND invoke the callback.
+         instance->UnLockNodeList();
 
          break;
 
@@ -286,7 +285,7 @@ void * XrdOucAvahiBonjour::BrowseEventLoopThread(void * context)
    }
 
    // Allocate a new client
-   callbackID->client = avahi_client_new(avahi_simple_poll_get(simple_poll), (AvahiClientFlags)0, ClientReply, NULL, &error);
+   callbackID->client = avahi_client_new(avahi_simple_poll_get(simple_poll), (AvahiClientFlags)0, ClientReply, callbackID, &error);
 
    // Check wether creating the client object succeeded
    if (!callbackID->client) {
@@ -306,6 +305,8 @@ void * XrdOucAvahiBonjour::BrowseEventLoopThread(void * context)
    // Run the main loop
    avahi_simple_poll_loop(simple_poll);
 
+   XrdLog.Emsg("OucBonjour", "Event loop thread terminated abnormally");
+
    return NULL; // Thread ends.
 }
 
@@ -347,12 +348,14 @@ void XrdOucAvahiBonjour::ResolveReply(AvahiServiceResolver *r,
                                       void* userdata)
 {
    XrdOucBonjourSubscribedEntry * callbackID;
-
+   XrdOucBonjourResolutionEntry * toResolve;
+   XrdOucAvahiBonjour *instance;
    AvahiStringList * iterator;
    char * key, * value, address_str[AVAHI_ADDRESS_STR_MAX];
    size_t size;
 
-   callbackID = static_cast<XrdOucBonjourSubscribedEntry *>(userdata);
+   toResolve = static_cast<XrdOucBonjourResolutionEntry *>(userdata);
+   callbackID = toResolve->callbackID;
 
    switch (event) {
       case AVAHI_RESOLVER_FAILURE:
@@ -360,19 +363,21 @@ void XrdOucAvahiBonjour::ResolveReply(AvahiServiceResolver *r,
          break;
 
       case AVAHI_RESOLVER_FOUND:
-			// Check if the resolver gethostbyname() function supports mDNS lookups.
-			if (avahi_nss_support()) {
-         	// Copy the information of resolution results to the node since the
-				// name can be resolved.
-         	callbackID->node->SetHostName(host_name);
-			} else {
-				// Save the address directly to improve name resolving on nodes that
-				// do not have an Avahi-enabled DNS resolver.
-				avahi_address_snprint(address_str, sizeof(address_str), address);
-				callbackID->node->SetHostName(address_str);
-			}
+         instance = &XrdOucAvahiBonjour::getInstance();
+         instance->LockNodeList();
+	 // Check if the resolver gethostbyname() function supports mDNS lookups.
+	 if (avahi_nss_support()) {
+            // Copy the information of resolution results to the node since the
+	    // name can be resolved.
+            toResolve->node->SetHostName(host_name);
+         } else {
+	    // Save the address directly to improve name resolving on nodes that
+            // do not have an Avahi-enabled DNS resolver.
+	    avahi_address_snprint(address_str, sizeof(address_str), address);
+	    toResolve->node->SetHostName(address_str);
+	 }
          // Note that Avahi returns the port in host order.
-         callbackID->node->SetPort(port);
+         toResolve->node->SetPort(port);
 
          // Also, copy the TXT values by iterating through the list of data.
          iterator = txt;
@@ -380,7 +385,7 @@ void XrdOucAvahiBonjour::ResolveReply(AvahiServiceResolver *r,
             // Get data from the TXT record.
             avahi_string_list_get_pair(iterator, &key, &value, &size);
             // Add to the Bonjour record.
-            callbackID->node->GetBonjourRecord().AddTXTRecord(key, value);
+            toResolve->node->GetBonjourRecord().AddTXTRecord(key, value);
             // Free data after copy.
             avahi_free(key);
             if (value)
@@ -388,17 +393,25 @@ void XrdOucAvahiBonjour::ResolveReply(AvahiServiceResolver *r,
             // Go to the next TXT record.
             iterator = avahi_string_list_get_next(iterator);
          }
+
+         // Insert now that the node is completely resolved.
+         instance->ListOfNodes.push_back(toResolve->node);
+         instance->UnLockNodeList();
    }
 
    // We must free the resolver object since it was created just before calling
    // the resolver procedure.
    avahi_service_resolver_free(r);
 
-   // Invoke the callback.
-   callbackID->callback(callbackID->context);
+   // Also, we should free the resolver data wrapper in order to avoid leaks.
+   free(toResolve);
+
+   // Invoke the callback if everything were fine.
+   if (event == AVAHI_RESOLVER_FOUND)
+      callbackID->callback(callbackID->context);
 }
 
-int XrdOucAvahiBonjour::ResolveNodeInformation(XrdOucBonjourNode * nodeInfo)
+int XrdOucAvahiBonjour::ResolveNodeInformation(XrdOucBonjourResolutionEntry * nodeAndCallback)
 {
    // With Avahi, the resolution must be done just after the discovery and
    // on-demand resolution is not supported.
diff --git a/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAvahiBonjour.hh b/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAvahiBonjour.hh
index f554dae0d41..53429d89f18 100644
--- a/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAvahiBonjour.hh
+++ b/net/xrootd/src/xrootd/src/XrdOuc/XrdOucAvahiBonjour.hh
@@ -129,7 +129,7 @@ public:
    // and port. It is important to use the resolution by-demand since the list
    // may not contain updated information due to the use of highly dynamical
    // DHCP and APIPA addresses.
-   int ResolveNodeInformation(XrdOucBonjourNode * nodeInfo);
+   int ResolveNodeInformation(XrdOucBonjourResolutionEntry * nodeAndCallback);
 
    // Accessor to get the singleton instance.
    static XrdOucAvahiBonjour &getInstance();
diff --git a/net/xrootd/src/xrootd/src/XrdOuc/XrdOucBonjour.cc b/net/xrootd/src/xrootd/src/XrdOuc/XrdOucBonjour.cc
index e2b9b0ba7e6..781b7307043 100644
--- a/net/xrootd/src/xrootd/src/XrdOuc/XrdOucBonjour.cc
+++ b/net/xrootd/src/xrootd/src/XrdOuc/XrdOucBonjour.cc
@@ -132,6 +132,7 @@ XrdOucBonjourRecord & XrdOucBonjourRecord::operator=(const XrdOucBonjourRecord &
 void XrdOucBonjourRecord::Print() const
 {
    XrdLog.Say("INFO: Bonjour RECORD = ", GetServiceName(), GetRegisteredType(), GetReplyDomain());
+   XrdLog.Say("INFO: Bonjour TXT = ", GetTXTRecordData());
 }
 
 void XrdOucBonjourRecord::AddRawTXTRecord(const char * rawData)
@@ -220,9 +221,10 @@ XrdOucBonjourNode & XrdOucBonjourNode::operator=(const XrdOucBonjourNode &other)
 
 void XrdOucBonjourNode::Print() const
 {
-   char port[6];
-   snprintf(port, 6, "%d", GetPort());
-   XrdLog.Say("INFO: Bonjour NODE = ", GetHostName(), ":", port);
+   char port[36];
+   snprintf(port, 36, "%d (%p)", GetPort(), this);
+   const char *host = GetHostName() ? GetHostName() : "<empty>";
+   XrdLog.Say("INFO: Bonjour NODE = ", host, ":", port);
    GetBonjourRecord().Print();
 }
 
diff --git a/net/xrootd/src/xrootd/src/XrdOuc/XrdOucBonjour.hh b/net/xrootd/src/xrootd/src/XrdOuc/XrdOucBonjour.hh
index 27ccab2ca6a..752d927c88b 100644
--- a/net/xrootd/src/xrootd/src/XrdOuc/XrdOucBonjour.hh
+++ b/net/xrootd/src/xrootd/src/XrdOuc/XrdOucBonjour.hh
@@ -58,10 +58,15 @@ typedef struct XrdOucBonjourSubscribedEntry {
    // Used to carry the Avahi browser information.
 #if defined(__linux__)
    AvahiClient * client;
-   XrdOucBonjourNode * node;
 #endif
 } XrdOucBonjourSubscribedEntry;
 
+// Used to make the resolution dinamycally.
+typedef struct XrdOucBonjourResolutionEntry {
+   XrdOucBonjourNode * node;
+   XrdOucBonjourSubscribedEntry * callbackID;
+} XrdOucBonjourResolutionEntry;
+
 /******************************************************************************/
 /*                        B o n j o u r   r e c o r d                         */
 /******************************************************************************/
@@ -246,7 +251,7 @@ public:
    // and port. It is important to use the resolution by-demand since the list
    // may not contain updated information due to the use of highly dynamical
    // DHCP and APIPA addresses.
-   virtual int ResolveNodeInformation(XrdOucBonjourNode * nodeInfo) = 0;
+   virtual int ResolveNodeInformation(XrdOucBonjourResolutionEntry * nodeAndCallback) = 0;
 
    // Returns the current list of discovered nodes through the Bonjour local
    // mDNS. This list cannot be modified by clients of the class.
diff --git a/net/xrootd/src/xrootd/src/XrdVersion.hh b/net/xrootd/src/xrootd/src/XrdVersion.hh
index eb9918f283b..ed67811ce71 100644
--- a/net/xrootd/src/xrootd/src/XrdVersion.hh
+++ b/net/xrootd/src/xrootd/src/XrdVersion.hh
@@ -1,7 +1,7 @@
 // $Id$
 #ifndef __XRD_VERSION_H__
 #define __XRD_VERSION_H__
-#define XrdVERSION  "v20100913-0630-root-4"
+#define XrdVERSION  "v20100913-0630-root-5"
 #if XrdDEBUG
 #define XrdVSTRING XrdVERSION "_dbg"
 #else
-- 
GitLab