SoupTransportAgent: require libsoup 2.42, no deprecated methods

This allows us to get rid of deprecated function calls. We no longer
need to set a default proxy either, the newer libsoup does that itself
by default
(https://developer.gnome.org/libsoup/stable/libsoup-session-porting.html).

Not mentioned in the porting guide is that the based soup session now
has a 60s timeout by default. We don't want to time out.

We also need to quit the event loop explicitly when timing out. Somehow
that wasn't necessary before.

When using the generic SoupSession, it is no longer guaranteed that canceling
an operation invokes the callbacks before returning. Therefore we have to be
prepared for callbacks occuring after destructing the SoupTransportAgent. This
is achieved by storing all SoupTransportAgent in a boost::shared_ptr and
letting the instance know about that with a weak_ptr, which then can be used
by the callbacks.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>

5 files changed, 56 insertions, 39 deletions

diff --git a/configure.ac b/configure.ac
index 4b4da576..83a29485 100644
--- a/configure.ac
+++ b/configure.ac
@@ -231,12 +231,9 @@ else
     have_libcurl="no"
 fi
 
-PKG_CHECK_MODULES(LIBSOUP, libsoup-gnome-2.4,
-                  [have_libsoup="yes"
-                   AC_DEFINE(HAVE_LIBSOUP_SOUP_GNOME_FEATURES_H, 1, [enable GNOME specific libsoup])],
-                  [PKG_CHECK_MODULES(LIBSOUP, libsoup-2.4,
-                                     have_libsoup="yes",
-                                     have_libsoup="no")])
+PKG_CHECK_MODULES(LIBSOUP, libsoup-2.4 >= 2.42,
+                           have_libsoup="yes",
+                           have_libsoup="no")
 
 PKG_CHECK_MODULES(LIBOPENOBEX, openobex, have_obex="yes", have_obex="no")
 have_bluetooth="no"
diff --git a/src/backends/oauth2/oauth2.cpp b/src/backends/oauth2/oauth2.cpp
index 087fe52d..b19735fa 100644
--- a/src/backends/oauth2/oauth2.cpp
+++ b/src/backends/oauth2/oauth2.cpp
@@ -54,8 +54,7 @@ public:
         m_refreshToken(refreshToken)
     {
 #ifdef ENABLE_LIBSOUP
-        boost::shared_ptr<SoupTransportAgent> agent(new SoupTransportAgent(static_cast<GMainLoop *>(NULL)));
-        m_agent = agent;
+        m_agent = SoupTransportAgent::create(static_cast<GMainLoop *>(NULL));
 #elif defined(ENABLE_LIBCURL)
         m_agent = new CurlTransportAgent();
 #endif
diff --git a/src/syncevo/SoupTransportAgent.cpp b/src/syncevo/SoupTransportAgent.cpp
index fb5d2bd0..9dd7f581 100644
--- a/src/syncevo/SoupTransportAgent.cpp
+++ b/src/syncevo/SoupTransportAgent.cpp
@@ -25,28 +25,28 @@
 #include <libsoup/soup-status.h>
 #include <syncevo/Logging.h>
 
-#ifdef HAVE_LIBSOUP_SOUP_GNOME_FEATURES_H
-#include <libsoup/soup-gnome-features.h>
-#endif
-
 #include <syncevo/declarations.h>
 SE_BEGIN_CXX
 
+boost::shared_ptr<SoupTransportAgent> SoupTransportAgent::create(GMainLoop *loop)
+{
+    boost::shared_ptr<SoupTransportAgent> self(new SoupTransportAgent(loop));
+    self->m_self = self;
+    return self;
+}
+
 SoupTransportAgent::SoupTransportAgent(GMainLoop *loop) :
     m_verifySSL(false),
-    m_session(soup_session_async_new()),
+    m_session(soup_session_new_with_options("timeout", 0, (void *)NULL)),
     m_loop(loop ?
            g_main_loop_ref(loop) :
            g_main_loop_new(NULL, TRUE),
            "Soup main loop"),
     m_status(INACTIVE),
+    m_message(NULL),
     m_timeoutSeconds(0),
     m_response(0)
 {
-#ifdef HAVE_LIBSOUP_SOUP_GNOME_FEATURES_H
-    // use default GNOME proxy settings
-    soup_session_add_feature_by_type(m_session.get(), SOUP_TYPE_PROXY_RESOLVER_GNOME);
-#endif
 }
 
 SoupTransportAgent::~SoupTransportAgent()
@@ -138,12 +138,14 @@ void SoupTransportAgent::send(const char *data, size_t len)
     soup_message_set_request(message.get(), m_contentType.c_str(),
                              SOUP_MEMORY_TEMPORARY, data, len);
     m_status = ACTIVE;
+    // We just keep a pointer for the timeout, without owning the message.
+    m_message = message.get();
     if (m_timeoutSeconds) {
-        m_message = message.get();
-        m_timeoutEventSource = g_timeout_add_seconds(m_timeoutSeconds, TimeoutCallback, static_cast<gpointer> (this));
+        m_timeout.runOnce(m_timeoutSeconds,
+                          boost::bind(&SoupTransportAgent::handleTimeoutWrapper, m_self));
     }
     soup_session_queue_message(m_session.get(), message.release(),
-                               SessionCallback, static_cast<gpointer>(this));
+                               SessionCallback, new boost::weak_ptr<SoupTransportAgent>(m_self));
 }
 
 void SoupTransportAgent::cancel()
@@ -188,7 +190,6 @@ TransportAgent::Status SoupTransportAgent::wait(bool noReply)
         SE_THROW_EXCEPTION(TransportException, failure);
     }
 
-    m_timeoutEventSource.set(0);
     return m_status;
 }
 
@@ -208,18 +209,29 @@ void SoupTransportAgent::SessionCallback(SoupSession *session,
                                          SoupMessage *msg,
                                          gpointer user_data)
 {
-    static_cast<SoupTransportAgent *>(user_data)->HandleSessionCallback(session, msg);
+    // A copy of the weak_ptr was created for us, which we need to delete now.
+    boost::weak_ptr<SoupTransportAgent> *self = static_cast< boost::weak_ptr<SoupTransportAgent> *>(user_data);
+    boost::shared_ptr<SoupTransportAgent> agent(self->lock());
+    delete self;
+
+    if (agent.get()) {
+        agent->HandleSessionCallback(session, msg);
+    }
 }
 
 void SoupTransportAgent::HandleSessionCallback(SoupSession *session,
                                                SoupMessage *msg)
 {
+    // Message is no longer pending, so timeout no longer needed either.
+    m_message = NULL;
+    m_timeout.deactivate();
+
     // keep a reference to the data 
     m_responseContentType = "";
     if (msg->response_body) {
         m_response = soup_message_body_flatten(msg->response_body);
-        const char *soupContentType = soup_message_headers_get(msg->response_headers,
-                                                               "Content-Type");
+        const char *soupContentType = soup_message_headers_get_one(msg->response_headers,
+                                                                   "Content-Type");
         if (soupContentType) {
             m_responseContentType = soupContentType;
         }
@@ -246,19 +258,24 @@ void SoupTransportAgent::HandleSessionCallback(SoupSession *session,
     g_main_loop_quit(m_loop.get());
 }
 
-gboolean SoupTransportAgent::processCallback()
+void SoupTransportAgent::handleTimeout()
 {
-    //stop the message processing and mark status as timeout
-    guint message_status = SOUP_STATUS_CANCELLED;
-    soup_session_cancel_message(m_session.get(), m_message, message_status);
-    m_status = TIME_OUT;
-    return FALSE;
+    // Stop the message processing and mark status as timeout, if message is really still
+    // pending.
+    if (m_message) {
+        guint message_status = SOUP_STATUS_CANCELLED;
+        soup_session_cancel_message(m_session.get(), m_message, message_status);
+        g_main_loop_quit(m_loop.get());
+        m_status = TIME_OUT;
+    }
 }
 
-gboolean SoupTransportAgent::TimeoutCallback(gpointer transport)
+void SoupTransportAgent::handleTimeoutWrapper(const boost::weak_ptr<SoupTransportAgent> &agent)
 {
-    SoupTransportAgent * sTransport = static_cast<SoupTransportAgent *>(transport);
-    return sTransport->processCallback();
+    boost::shared_ptr<SoupTransportAgent> self(agent.lock());
+    if (self.get()) {
+        self->handleTimeout();
+    }
 }
 
 SE_END_CXX
diff --git a/src/syncevo/SoupTransportAgent.h b/src/syncevo/SoupTransportAgent.h
index b4eba8ae..d43fd408 100644
--- a/src/syncevo/SoupTransportAgent.h
+++ b/src/syncevo/SoupTransportAgent.h
@@ -26,9 +26,12 @@
 
 #include <syncevo/TransportAgent.h>
 #include <syncevo/SmartPtr.h>
+#include <syncevo/timeout.h>
 #include <libsoup/soup.h>
 #include <glib.h>
 
+#include <boost/weak_ptr.hpp>
+
 #include <syncevo/declarations.h>
 SE_BEGIN_CXX
 
@@ -55,7 +58,7 @@ class SoupTransportAgent : public HTTPTransportAgent
      *                  transport will increase the reference count;
      *                  if NULL a new loop in the default context is used
      */
-    SoupTransportAgent(GMainLoop *loop = NULL);
+    static boost::shared_ptr<SoupTransportAgent> create(GMainLoop *loop = NULL);
     ~SoupTransportAgent();
 
     virtual void setURL(const std::string &url);
@@ -72,8 +75,8 @@ class SoupTransportAgent : public HTTPTransportAgent
     virtual Status wait(bool noReply = false);
     virtual void getReply(const char *&data, size_t &len, std::string &contentType);
     virtual void setTimeout(int seconds);
-    gboolean processCallback();
  private:
+    boost::weak_ptr<SoupTransportAgent> m_self;
     std::string m_proxyUser;
     std::string m_proxyPassword;
     std::string m_cacerts;
@@ -86,11 +89,10 @@ class SoupTransportAgent : public HTTPTransportAgent
     std::string m_failure;
 
     SoupMessage *m_message;
-    GLibEvent m_timeoutEventSource;
+    Timeout m_timeout;
     int m_timeoutSeconds;
 
-    /** This function is called regularly to detect timeout */
-    static gboolean TimeoutCallback (gpointer data);
+    SoupTransportAgent(GMainLoop *loop);
 
     /** response, copied from SoupMessage */
     eptr<SoupBuffer, SoupBuffer, GLibUnref> m_response;
@@ -102,6 +104,8 @@ class SoupTransportAgent : public HTTPTransportAgent
                                 gpointer user_data);
     void HandleSessionCallback(SoupSession *session,
                                SoupMessage *msg);
+    void handleTimeout();
+    static void handleTimeoutWrapper(const boost::weak_ptr<SoupTransportAgent> &agent);
 };
 
 SE_END_CXX
diff --git a/src/syncevo/SyncContext.cpp b/src/syncevo/SyncContext.cpp
index c522d9f4..3bef32f3 100644
--- a/src/syncevo/SyncContext.cpp
+++ b/src/syncevo/SyncContext.cpp
@@ -1682,7 +1682,7 @@ boost::shared_ptr<TransportAgent> SyncContext::createTransportAgent(void *gmainl
     } else if (boost::starts_with(url, "http://") ||
         boost::starts_with(url, "https://")) {
 #ifdef ENABLE_LIBSOUP
-        boost::shared_ptr<SoupTransportAgent> agent(new SoupTransportAgent(static_cast<GMainLoop *>(gmainloop)));
+        boost::shared_ptr<SoupTransportAgent> agent(SoupTransportAgent::create(static_cast<GMainLoop *>(gmainloop)));
         agent->setConfig(*this);
         InitializeTransport(agent, timeout);
         return agent;