浏览代码

HPCC-14140 Potential thread and socket leak from cacheFileConnect

Remove weirdly-coded workaround for (since fixed) inability to set timeout
when connecting to remote files.

Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>
Richard Chapman 9 年之前
父节点
当前提交
34d8b9c788

+ 0 - 11
common/remote/rmtfile.cpp

@@ -568,17 +568,6 @@ void setDaliServixSocketCaching(bool set)
     clientSetDaliServixSocketCaching(set);
 }
 
-void cacheFileConnect(IFile *file,unsigned timeout)
-{
-    RemoteFilename rfn;
-    rfn.setRemotePath(file->queryFilename());
-    if (!rfn.isLocal()&&!rfn.isNull()) {
-        SocketEndpoint ep = rfn.queryEndpoint();
-        if (ep.port)
-            clientCacheFileConnect(ep,timeout);
-    }
-}
-
 void disconnectRemoteFile(IFile *file)
 {
     clientDisconnectRemoteFile(file);

+ 0 - 1
common/remote/rmtfile.hpp

@@ -51,7 +51,6 @@ extern REMOTE_API IDaFileSrvHook *queryDaFileSrvHook();
 extern REMOTE_API unsigned short getDaliServixPort();  // assumed just the one for now
 extern REMOTE_API void setCanAccessDirectly(RemoteFilename & file,bool set);
 extern REMOTE_API void setDaliServixSocketCaching(bool set);
-extern REMOTE_API void cacheFileConnect(IFile *file,unsigned timeout);
 extern REMOTE_API bool canAccessDirectly(const RemoteFilename & file);
 extern REMOTE_API IFile *createDaliServixFile(const RemoteFilename & file);
 extern REMOTE_API bool testDaliServixPresent(const IpAddress &ip);

+ 0 - 52
common/remote/sockfile.cpp

@@ -2010,58 +2010,6 @@ public:
     }
 };
 
-void clientCacheFileConnect(SocketEndpoint &_ep,unsigned timeout)
-{
-    if (!timeout) {
-        SocketEndpoint ep(_ep);
-        setDafsEndpointPort(ep);
-        Owned<CRemoteFile> cfile = new CRemoteFile(ep, "null");
-        cfile->connect();
-        return; // frees file and adds its socket to cache
-    }
-    // timeout needed so start a thread (that may become orphaned)
-    class cThread: public Thread
-    {
-        SocketEndpoint ep;
-    public:
-        cThread(SocketEndpoint &_ep)
-            : Thread("cacheFileConnect")
-        {
-            ep = _ep;
-        }
-        int run()
-        {
-            try {
-                clientCacheFileConnect(ep,0);
-            }
-            catch (IException *e) {
-                CriticalBlock block(sect);
-                except.setown(e);
-            }
-            return 0;
-        }
-        Owned<IException> except;
-        CriticalSection sect;
-        
-    } *thread;
-    thread = new cThread(_ep);
-    thread->start();
-    IException *e =NULL;
-    if (!thread->join(timeout)) {
-        StringBuffer msg("Timed out connecting to ");
-        _ep.getUrlStr(msg);
-        e =  createDafsException(RFSERR_AuthenticateFailed,msg.str());
-    }
-    {
-        CriticalBlock block(thread->sect);
-        if (!e&&thread->except) 
-            e = thread->except.getClear();
-    }
-    thread->Release();
-    if (e)
-        throw e;
-}
-
 void clientAddSocketToCache(SocketEndpoint &ep,ISocket *socket)
 {
     CriticalBlock block(CConnectionTable::crit); 

+ 0 - 1
common/remote/sockfile.hpp

@@ -79,7 +79,6 @@ extern void setDafsLocalMountRedirect(const IpAddress &ip,const char *dir,const
 
 // client only
 extern void clientSetDaliServixSocketCaching(bool set);
-extern void clientCacheFileConnect(SocketEndpoint &ep,unsigned timeout);
 extern void clientDisconnectRemoteFile(IFile *file);
 extern void clientDisconnectRemoteIoOnExit(IFileIO *fileio,bool set);
 

+ 14 - 0
initfiles/componentfiles/configxml/roxie.xsd.in

@@ -706,6 +706,13 @@
         </xs:appinfo>
       </xs:annotation>
     </xs:attribute>
+    <xs:attribute name="remoteConnectTimeout" type="xs:integer" use="optional" default="10000">
+      <xs:annotation>
+        <xs:appinfo>
+          <tooltip>Timeout (in ms) for remote file connections</tooltip>
+        </xs:appinfo>
+      </xs:annotation>
+    </xs:attribute>
     <xs:attribute name="remoteFilesExpire" type="xs:integer" use="optional" default="3600000">
       <xs:annotation>
         <xs:appinfo>
@@ -713,6 +720,13 @@
         </xs:appinfo>
       </xs:annotation>
     </xs:attribute>
+    <xs:attribute name="remoteReadTimeout" type="xs:integer" use="optional" default="0">
+      <xs:annotation>
+        <xs:appinfo>
+          <tooltip>Timeout (in ms) for remote file reads</tooltip>
+        </xs:appinfo>
+      </xs:annotation>
+    </xs:attribute>
     <xs:attribute name="serverThreads" type="xs:nonNegativeInteger" use="optional" default="30">
       <xs:annotation>
         <xs:appinfo>

+ 2 - 11
roxie/ccd/ccdfile.cpp

@@ -217,7 +217,7 @@ public:
         {
             StringBuffer filesTried;
             unsigned tries = 0;
-            bool firstTime = true; // first time try the "fast / cache" way - if that fails - try original away - if that still fails, error
+            bool firstTime = true;
             RoxieFileStatus fileStatus = FileNotFound;
 
             loop
@@ -226,7 +226,7 @@ public:
                     currentIdx = 0;
                 if (tries==sources.length())
                 {
-                    if (firstTime)  // if first time - reset and try again - non cache way
+                    if (firstTime)  // if first time - reset and try again
                     {
                         firstTime = false;
                         tries = 0;
@@ -245,14 +245,6 @@ public:
                         throw MakeStringException(ROXIE_FILE_OPEN_FAIL, "Pretending to fail on an open");
 #endif
                     IFile *f = &sources.item(currentIdx);
-                    if (firstTime)
-                        cacheFileConnect(f, dafilesrvLookupTimeout);  // set timeout to 10 seconds
-                    else
-                    {
-                        if (traceLevel > 10)
-                            DBGLOG("Looking for file using non-cached file open");
-                    }
-
                     fileStatus = queryFileCache().fileUpToDate(f, fileSize, fileDate, crc, isCompressed, false);
                     if (fileStatus == FileIsValid)
                     {
@@ -605,7 +597,6 @@ class CRoxieFileCache : public CInterface, implements ICopyFileProgress, impleme
             IFile *f;
         } autoDisconnector(f, autoDisconnect);
 
-        cacheFileConnect(f, dafilesrvLookupTimeout);  // set timeout to 10 seconds
         if (f->exists())
         {
             // only check size if specified

+ 3 - 0
roxie/ccd/ccdmain.cpp

@@ -678,6 +678,9 @@ int STARTQUERY_API start_query(int argc, const char *argv[])
         lowTimeout = topology->getPropInt("@lowTimeout", 10000);
         highTimeout = topology->getPropInt("@highTimeout", 2000);
         slaTimeout = topology->getPropInt("@slaTimeout", 2000);
+        unsigned remoteConnectTimeout = topology->getPropInt("@remoteConnectTimeout", 10000);
+        unsigned remoteReadTimeout = topology->getPropInt("@remoteReadTimeout", 0);
+        setRemoteFileTimeouts(remoteConnectTimeout, remoteReadTimeout);
         parallelLoopFlowLimit = topology->getPropInt("@parallelLoopFlowLimit", 100);
         perChannelFlowLimit = topology->getPropInt("@perChannelFlowLimit", 10);
         copyResources = topology->getPropBool("@copyResources", true);