Avoid potential memory leaks from callback objects.

Laboriously emulate from XrdCl the cases where we own the "response" pointer and those where we do not. I suspect this may have been leaking a few bytes per file open - not going to kill the memory budget, but likely good for those using valgrind!
cms-sw · Jan 27, 2016 · 82c58cb · 82c58cb
1 parent d225325
commit 82c58cb
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 2 deletions.
diff --git a/Utilities/XrdAdaptor/plugins/XrdStorageMaker.cc b/Utilities/XrdAdaptor/plugins/XrdStorageMaker.cc
@@ -20,7 +20,9 @@ class MakerResponseHandler : public XrdCl::ResponseHandler
     virtual void HandleResponse( XrdCl::XRootDStatus *status,
                                  XrdCl::AnyObject    *response )
     {
-        if (response) delete response;
+        // Note: Prepare call has a response object.
+        delete response;
+        delete status;
     }
 
 };

diff --git a/Utilities/XrdAdaptor/src/XrdRequestManager.cc b/Utilities/XrdAdaptor/src/XrdRequestManager.cc
@@ -75,8 +75,12 @@ class SendMonitoringInfoHandler : boost::noncopyable, public XrdCl::ResponseHand
         {
             XrdCl::Buffer *buffer = nullptr;
             response->Get(buffer);
+            response->Set(static_cast<int*>(nullptr));
             delete buffer;
         }
+        // Send Info has a response object; we must delete it.
+        delete response;
+        delete status;
     }
 };
 
@@ -1062,6 +1066,7 @@ XrdAdaptor::RequestManager::OpenHandler::~OpenHandler()
 void
 XrdAdaptor::RequestManager::OpenHandler::HandleResponseWithHosts(XrdCl::XRootDStatus *status_ptr, XrdCl::AnyObject *, XrdCl::HostList *hostList_ptr)
 {
+  // NOTE: as in XrdCl::File (synchronous), we ignore the response object.
   std::shared_ptr<Source> source;
   std::unique_ptr<XrdCl::XRootDStatus> status(status_ptr);
   std::unique_ptr<XrdCl::HostList> hostList(hostList_ptr);

diff --git a/Utilities/XrdAdaptor/src/XrdSource.cc b/Utilities/XrdAdaptor/src/XrdSource.cc
@@ -58,14 +58,15 @@ class DelayedClose : boost::noncopyable, public XrdCl::ResponseHandler
 
     virtual void HandleResponseWithHosts(XrdCl::XRootDStatus *status, XrdCl::AnyObject *response, XrdCl::HostList *hostList) override
     {
-        if (!status->IsOK())
+        if (status && !status->IsOK())
         {
 
             edm::LogWarning("XrdFileWarning") << "Source delayed close failed with error '" << status->ToStr()
                 << "' (errno=" << status->errNo << ", code=" << status->code << ", server=" << m_id << ", site=" << m_site << ")";
         }
         delete status;
         delete hostList;
+        // NOTE: we do not delete response (copying behavior from XrdCl).
         delete this;
     }