fix timeouts for mh_execm filters

src/internfile/mh_exec.cpp

@@ -34,29 +34,39 @@
 
 using namespace std;
 
-// This is called periodically by ExeCmd when it is waiting for data,
+MimeHandlerExec::MimeHandlerExec(RclConfig *cnf, const std::string& id)
-// or when it does receive some. We may choose to interrupt the
+    : RecollFilter(cnf, id), missingHelper(false), m_filtermaxseconds(900),
-// command.
+      m_filtermaxmbytes(0) 
-class MEAdv : public ExecCmdAdvise {
+{
-public:
+    m_config->getConfParam("filtermaxseconds", &m_filtermaxseconds);
-    MEAdv(int maxsecs) : m_filtermaxseconds(maxsecs) {m_start = time(0L);}
+    m_config->getConfParam("filtermaxmbytes", &m_filtermaxmbytes);
-    void newData(int n) {
+}
-        LOGDEB1(("MHExec:newData(%d)\n", n));
-        if (m_filtermaxseconds > 0 && 
-            time(0L) - m_start > m_filtermaxseconds) {
-            LOGERR(("MimeHandlerExec: filter timeout (%d S)\n",
-                    m_filtermaxseconds));
-            CancelCheck::instance().setCancel();
-        }
-        // If a cancel request was set by the signal handler (or by us
-        // just above), this will raise an exception. Another approach
-        // would be to call ExeCmd::setCancel().
-	CancelCheck::instance().checkCancel();
-    }
-    time_t m_start;
-    int m_filtermaxseconds;
-};
 
+MEAdv::MEAdv(int maxsecs) 
+  : m_filtermaxseconds(maxsecs) 
+{
+    m_start = time(0L);
+}
+void MEAdv::reset()
+{
+    m_start = time(0L);
+
+}
+
+void MEAdv::newData(int n) 
+{
+    LOGDEB2(("MHExec:newData(%d)\n", n));
+    if (m_filtermaxseconds > 0 && 
+        time(0L) - m_start > m_filtermaxseconds) {
+        LOGERR(("MimeHandlerExec: filter timeout (%d S)\n",
+                m_filtermaxseconds));
+        CancelCheck::instance().setCancel();
+    }
+    // If a cancel request was set by the signal handler (or by us
+    // just above), this will raise an exception. Another approach
+    // would be to call ExeCmd::setCancel().
+    CancelCheck::instance().checkCancel();
+}
 
 bool MimeHandlerExec::skip_to_document(const string& ipath) 
 {
@@ -77,11 +87,6 @@ bool MimeHandlerExec::next_document()
 	return false;
     }
 
-    int filtermaxseconds = 900;
-    m_config->getConfParam("filtermaxseconds", &filtermaxseconds);
-    int filtermaxmbytes = 0;
-    m_config->getConfParam("filtermaxmbytes", &filtermaxmbytes);
-
     if (params.empty()) {
 	// Hu ho
 	LOGERR(("MimeHandlerExec::mkDoc: empty params\n"));
@@ -102,12 +107,12 @@ bool MimeHandlerExec::next_document()
     string& output = m_metaData[cstr_dj_keycontent];
     output.erase();
     ExecCmd mexec;
-    MEAdv adv(filtermaxseconds);
+    MEAdv adv(m_filtermaxseconds);
     mexec.setAdvise(&adv);
     mexec.putenv("RECOLL_CONFDIR", m_config->getConfDir());
     mexec.putenv(m_forPreview ? "RECOLL_FILTER_FORPREVIEW=yes" :
 		"RECOLL_FILTER_FORPREVIEW=no");
-    mexec.setrlimit_as(filtermaxmbytes);
+    mexec.setrlimit_as(m_filtermaxmbytes);
 
     int status;
     try {

src/internfile/mh_exec.h

@@ -19,10 +19,9 @@
 
 #include <string>
 #include <vector>
-using std::vector;
-using std::string;
 
 #include "mimehandler.h"
+#include "execmd.h"
 
 /** 
  * Turn external document into internal one by executing an external filter.
@@ -45,28 +44,33 @@ class MimeHandlerExec : public RecollFilter {
 
     // Parameters: this has been built by our creator, from config file 
     // data. We always add the file name at the end before actual execution
-    vector<string> params;
+    std::vector<std::string> params;
     // Filter output type. The default for ext. filters is to output html, 
     // but some don't, in which case the type is defined in the config.
-    string cfgFilterOutputMtype;
+    std::string cfgFilterOutputMtype;
     // Output character set if the above type is not text/html. For
     // those filters, the output charset has to be known: ie set by a command
     // line option.
-    string cfgFilterOutputCharset; 
+    std::string cfgFilterOutputCharset; 
     bool missingHelper;
+    // Resource management values
+    int m_filtermaxseconds;
+    int m_filtermaxmbytes;
     ////////////////
 
-    MimeHandlerExec(RclConfig *cnf, const string& id) 
+    MimeHandlerExec(RclConfig *cnf, const std::string& id);
-	: RecollFilter(cnf, id), missingHelper(false) 
+
-    {}
+    virtual bool set_document_file(const std::string& mt, 
-    virtual bool set_document_file(const string& mt, const string &file_path) {
+                                   const std::string &file_path) {
 	RecollFilter::set_document_file(mt, file_path);
 	m_fn = file_path;
 	m_havedoc = true;
 	return true;
     }
+
     virtual bool next_document();
-    virtual bool skip_to_document(const string& ipath); 
+    virtual bool skip_to_document(const std::string& ipath); 
+
     virtual void clear() {
 	m_fn.erase(); 
 	m_ipath.erase();
@@ -74,17 +78,36 @@ class MimeHandlerExec : public RecollFilter {
     }
 
 protected:
-    string m_fn;
+    std::string m_fn;
-    string m_ipath;
+    std::string m_ipath;
 
     // Set up the character set metadata fields and possibly transcode
     // text/plain output. 
     // @param charset when called from mh_execm, a possible explicit
     //       value from the filter (else the data will come from the config)
-    virtual void handle_cs(const string& mt, const string& charset = string());
+    virtual void handle_cs(const std::string& mt, 
+                           const std::string& charset = std::string());
 
 private:
     virtual void finaldetails();
 };
 
+
+// This is called periodically by ExeCmd when it is waiting for data,
+// or when it does receive some. We may choose to interrupt the
+// command.
+class MEAdv : public ExecCmdAdvise {
+public:
+    MEAdv(int maxsecs = 900);
+    // Reset start time to now
+    void reset();
+    void setmaxsecs(int maxsecs) {
+        m_filtermaxseconds = maxsecs;
+    }
+    void newData(int n);
+private:
+    time_t m_start;
+    int m_filtermaxseconds;
+};
+
 #endif /* _MH_EXEC_H_INCLUDED_ */

src/internfile/mh_execm.cpp

@@ -47,9 +47,6 @@ bool MimeHandlerExecMultiple::startCmd()
     // Command name
     string cmd = params.front();
     
-    int filtermaxmbytes = 0;
-    m_config->getConfParam("filtermaxmbytes", &filtermaxmbytes);
-
     m_maxmemberkb = 50000;
     m_config->getConfParam("membermaxkbs", &m_maxmemberkb);
     ostringstream oss;
@@ -60,7 +57,9 @@ bool MimeHandlerExecMultiple::startCmd()
     m_cmd.putenv(m_forPreview ? "RECOLL_FILTER_FORPREVIEW=yes" :
 		"RECOLL_FILTER_FORPREVIEW=no");
 
-    m_cmd.setrlimit_as(filtermaxmbytes);
+    m_cmd.setrlimit_as(m_filtermaxmbytes);
+    m_adv.setmaxsecs(m_filtermaxseconds);
+    m_cmd.setAdvise(&m_adv);
 
     // Build parameter list: delete cmd name
     vector<string>myparams(params.begin() + 1, params.end());
@@ -198,6 +197,8 @@ bool MimeHandlerExecMultiple::next_document()
         return false;
     }
 
+    m_adv.reset();
+
     // Read answer (multiple elements)
     LOGDEB1(("MHExecMultiple: reading answer\n"));
     bool eofnext_received = false;
@@ -209,7 +210,13 @@ bool MimeHandlerExecMultiple::next_document()
     string charset;
     for (int loop=0;;loop++) {
         string name, data;
-        if (!readDataElement(name, data)) {
+        try {
+            if (!readDataElement(name, data)) {
+                m_cmd.zapChild();
+                return false;
+            }
+        } catch (CancelExcept) {
+            LOGINFO(("MHExecMultiple: timeout or interrupt\n"));
             m_cmd.zapChild();
             return false;
         }

src/internfile/mh_execm.h

@@ -120,6 +120,7 @@ private:
     bool readDataElement(string& name, string& data);
     bool m_filefirst;
     int  m_maxmemberkb;
+    MEAdv m_adv;
 };
 
 #endif /* _MH_EXECM_H_INCLUDED_ */

src/qtgui/recoll.pro.in

@@ -1,6 +1,8 @@
 TEMPLATE        = app
 LANGUAGE        = C++
 
+DEFINES += BUILDING_RECOLL
+
 @QMAKE_ENABLE_WEBKIT@ QT += webkit
 @QMAKE_DISABLE_WEBKIT@ QMAKE_CXXFLAGS += -DRESLIST_TEXTBROWSER -DSNIPPETS_TEXTBROWSER
 

src/utils/execmd.cpp

@@ -907,8 +907,22 @@ int ExecCmd::getline(string& data)
     }
     const int BS = 1024;
     char buf[BS];
-    int n = con->getline(buf, BS);
+    int timeosecs = m->m_timeoutMs / 1000;
+    if (timeosecs == 0)
+        timeosecs = 1;
+
+    // Note that we only go once through here, except in case of
+    // timeout, which is why I think that the goto is more expressive
+    // than a loop
+again:
+    int n = con->getline(buf, BS, timeosecs);
     if (n < 0) {
+        if (con->timedout()) {
+            LOGDEB(("ExecCmd::getline: timeout\n"));
+            if (m->m_advise)
+                m->m_advise->newData(0);
+            goto again;
+        }
 	LOGERR(("ExecCmd::getline: error\n"));
     } else if (n > 0) {
 	data.append(buf, n);

src/utils/netcon.cpp

@@ -117,8 +117,7 @@ int Netcon::select1(int fd, int timeo, int write)
         ret = select(fd+1, &rd, 0, 0, &tv);
     }
     if (!FD_ISSET(fd, &rd)) {
-        LOGERR(("Netcon::select1: fd not ready after select ??\n"));
+        LOGDEB2(("Netcon::select1: fd %d timeout\n",fd));
-        return -1;
     }
     return ret;
 }