Second sketch for mechanism to cancel DcmSCU negotiateAssociation before connectionTimeout elapses

dcmnet/include/dcmtk/dcmnet/dul.h

@@ -202,7 +202,10 @@ typedef struct {
     UserIdentityNegotiationSubItemAC *ackUserIdentNeg;
 
     OFBool useSecureLayer;
-    Sint32 tcpConnectTimeout;
+    Sint32 tcpConnectTimeout;           // TCP connect timeout in seconds when requesting associations
+    Sint32 tcpPollInterval;             // Duration in seconds of each socket poll/select call during TCP connection phase. Maximum valid value is tcpConnectTimeout.
+    void*  tcpCancelContext;            // User defined data that is passed on to tcpConnectCancelled()
+    bool (*tcpConnectCanceled)(void*);  // Called after each socket poll/select attempt to enable canceling ongoing TCP connect before the TCP connect timeout has elapsed
 }   DUL_ASSOCIATESERVICEPARAMETERS;
 
 /** Enum describing the possible role settings for role negotiation sub items.

dcmnet/include/dcmtk/dcmnet/scu.h

@@ -185,6 +185,13 @@ class DCMTK_DCMNET_EXPORT RetrieveResponse : public QRResponse
     RetrieveResponse& operator=(const RetrieveResponse& other);
 };
 
+class IDcmCancelToken
+{
+public:
+    virtual ~IDcmCancelToken() {}
+    virtual bool IsCanceled() const = 0;
+};
+
 /** Base class for implementing DICOM Service Class User functionality. The class offers
  *  support for negotiating associations and sending and receiving arbitrary DIMSE messages
  *  on that connection. DcmSCU has built-in C-ECHO support so derived classes do not have to
@@ -223,10 +230,16 @@ class DCMTK_DCMNET_EXPORT DcmSCU
     /** Negotiate association by using presentation contexts and parameters as defined by
      *  earlier function calls. If negotiation fails, there is no need to close the association
      *  or to do anything else with this class.
+     *  @param tcpCancelToken [in] Optional cancellation token which is checked periodically
+     *                             for cancellation. If the token is cancelled during TCP
+     *                             connect phase, the TCP connect attempts are stopped and this
+     *                             function returns failure.
      *  @return EC_Normal if negotiation was successful, otherwise error code.
-     *          NET_EC_AlreadyConnected if SCU is already connected.
+     *          NET_EC_AlreadyConnected if SCU is already connected. DULC_TCPINITERROR is returned
+     *          if the TCP connect attempt was cancelled before the connection was established
+     *
      */
-    virtual OFCondition negotiateAssociation();
+    virtual OFCondition negotiateAssociation(IDcmCancelToken* tcpCancelToken = NULL);
 
     /** After negotiation association, this call returns the first usable presentation context
      *  given the desired abstract syntax and transfer syntax
@@ -690,6 +703,15 @@ class DCMTK_DCMNET_EXPORT DcmSCU
      */
     void setConnectionTimeout(const Sint32 connectionTimeout);
 
+    /** Set the poll interval (short timeout) used when connecting to the SCP.
+     * While requesting an association, the TCP connection will be checked for successful
+     * connection with this timeout until the full connection timeout has been reached, or
+     * until the connection attempt has been cancelled.
+     *  @param pollInterval [in] poll interval in seconds, <= connectionTimeout).
+     *                           set to -1 to disable connect cancellation
+     */
+    void setTcpPollInterval(const Sint32 pollInterval);
+
     /** Set an association configuration file and profile to be used
      *  @param filename [in] File name of the association configuration file
      *  @param profile  [in] Profile inside the association negotiation file
@@ -786,6 +808,12 @@ class DCMTK_DCMNET_EXPORT DcmSCU
      */
     Sint32 getConnectionTimeout() const;
 
+    /** Gets the poll interval (short timeout) that defines how frequently the SCU will check
+     * for cancelled connect attempt.
+     * @return The poll interval (in seconds)
+     */
+    Sint32 getTcpPollInterval() const;
+
     /** Returns the storage directory used for storing objects received with C-STORE requests
      *  in the context of C-GET sessions. Default is empty string which refers to the current
      *  working directory.
@@ -1099,6 +1127,9 @@ class DCMTK_DCMNET_EXPORT DcmSCU
     /// TCP connection timeout (default: value of global dcmConnectionTimeout)
     Sint32 m_tcpConnectTimeout;
 
+    /// TCP poll interval (default: -1 (disabled))
+    Sint32 m_tcpPollInterval;
+
     /// Storage directory for objects received with C-STORE due to a running
     /// C-GET session. By default, the received objects are stored in the current
     /// working directory.

dcmnet/libsrc/assoc.cc

@@ -336,6 +336,9 @@ ASC_createAssociationParameters(T_ASC_Parameters ** params,
 
     (*params)->DULparams.useSecureLayer = OFFalse;
     (*params)->DULparams.tcpConnectTimeout = tcpConnectTimeout;
+    (*params)->DULparams.tcpPollInterval = -1;
+    (*params)->DULparams.tcpCancelContext = NULL;
+    (*params)->DULparams.tcpConnectCanceled = NULL;
     return EC_Normal;
 }
 

dcmnet/libsrc/dulfsm.cc

@@ -116,6 +116,8 @@ END_EXTERN_C
 #include "dcmtk/dcmnet/diutil.h"
 #include "dcmtk/dcmnet/helpers.h"
 #include "dcmtk/ofstd/ofsockad.h" /* for class OFSockAddr */
+#include "dcmtk/ofstd/oftimer.h"
+
 #include <ctime>
 
 
@@ -2303,34 +2305,47 @@ requestAssociationTCP(PRIVATE_NETWORKKEY ** network,
     if (rc < 0 && errno == EINPROGRESS)
 #endif
     {
+        rc = 0;
+        OFTimer connectTimer;
+        do {
 #ifndef DCMTK_HAVE_POLL
-        // we're in non-blocking mode. Prepare to wait for timeout.
-        fd_set fdSet;
-        FD_ZERO(&fdSet);
+            // we're in non-blocking mode. Prepare to wait for timeout.
+            fd_set fdSet;
+            FD_ZERO(&fdSet);
 #ifdef __MINGW32__
-        // on MinGW, FD_SET expects an unsigned first argument
-        FD_SET((unsigned int) s, &fdSet);
+            // on MinGW, FD_SET expects an unsigned first argument
+            FD_SET((unsigned int) s, &fdSet);
 #else
-        FD_SET(s, &fdSet);
+            FD_SET(s, &fdSet);
 #endif /* __MINGW32__ */
 #endif /* DCMTK_HAVE_POLL */
 
-        struct timeval timeout;
-        timeout.tv_sec = connectTimeout;
-        timeout.tv_usec = 0;
+            struct timeval timeout;
+            if (params->tcpPollInterval == -1)
+                timeout.tv_sec = connectTimeout;
+            else
+                timeout.tv_sec = params->tcpPollInterval;
+            timeout.tv_usec = 0;
+
+            if (params->tcpConnectCanceled && params->tcpConnectCanceled(params->tcpCancelContext)) {
+                // TCP connect attempt was canceled. Flag connection as timed out
+                rc = 0; 
+                break;
+            }
 
-        do {
+            do {
 #ifdef DCMTK_HAVE_POLL
-            struct pollfd pfd[] =
-            {
-                { s, POLLOUT, 0 }
-            };
-            rc = poll(pfd, 1, timeout.tv_sec*1000+(timeout.tv_usec/1000));
+                struct pollfd pfd[] =
+                {
+                    { s, POLLOUT, 0 }
+                };
+                rc = poll(pfd, 1, timeout.tv_sec*1000+(timeout.tv_usec/1000));
 #else
-            // the typecast is safe because Windows ignores the first select() parameter anyway
-            rc = select(OFstatic_cast(int, s + 1), NULL, &fdSet, NULL, &timeout);
+                // the typecast is safe because Windows ignores the first select() parameter anyway
+                rc = select(OFstatic_cast(int, s + 1), NULL, &fdSet, NULL, &timeout);
 #endif
-        } while (rc == -1 && OFStandard::getLastNetworkErrorCode().value() == DCMNET_EINTR);
+            } while (rc == -1 && OFStandard::getLastNetworkErrorCode().value() == DCMNET_EINTR);
+        } while (rc == 0 && connectTimer.getDiff() < connectTimeout);
 
         if (DCM_dcmnetLogger.isEnabledFor(OFLogger::DEBUG_LOG_LEVEL))
         {

dcmnet/libsrc/scu.cc

@@ -32,6 +32,15 @@
 #include <zlib.h> /* for zlibVersion() */
 #endif
 
+static bool CancelTcpConnect(void* context)
+{
+    if (!context)
+        return false;
+
+    const IDcmCancelToken* token = OFstatic_cast(const IDcmCancelToken*, context);
+    return token->IsCanceled();
+}
+
 DcmSCU::DcmSCU()
     : m_assoc(NULL)
     , m_net(NULL)
@@ -50,6 +59,7 @@ DcmSCU::DcmSCU()
     , m_dimseTimeout(0)
     , m_acseTimeout(30)
     , m_tcpConnectTimeout(dcmConnectionTimeout.get())
+    , m_tcpPollInterval(-1)
     , m_storageDir()
     , m_storageMode(DCMSCU_STORAGE_DISK)
     , m_verbosePCMode(OFFalse)
@@ -128,6 +138,9 @@ OFCondition DcmSCU::initNetwork()
         return cond;
     }
 
+    m_params->DULparams.tcpPollInterval = m_tcpPollInterval;
+    m_params->DULparams.tcpConnectCanceled = CancelTcpConnect;
+
     /* sets this application's title and the called application's title in the params */
     /* structure. The default values are "ANY-SCU" and "ANY-SCP". */
     ASC_setAPTitles(m_params, m_ourAETitle.c_str(), m_peerAETitle.c_str(), NULL);
@@ -258,7 +271,7 @@ OFCondition DcmSCU::initNetwork()
     return cond;
 }
 
-OFCondition DcmSCU::negotiateAssociation()
+OFCondition DcmSCU::negotiateAssociation(IDcmCancelToken* tcpCancelToken)
 {
     /* Return error if SCU is already connected */
     if (isConnected())
@@ -271,6 +284,9 @@ OFCondition DcmSCU::negotiateAssociation()
     else
         DCMNET_DEBUG("Request Parameters:" << OFendl << ASC_dumpParameters(tempStr, m_params, ASC_ASSOC_RQ));
 
+
+    m_params->DULparams.tcpCancelContext = tcpCancelToken;
+
     /* create association, i.e. try to establish a network connection to another */
     /* DICOM application. This call creates an instance of T_ASC_Association*. */
     DCMNET_INFO("Requesting Association");
@@ -2562,6 +2578,11 @@ void DcmSCU::setConnectionTimeout(const Sint32 connectionTimeout)
     m_tcpConnectTimeout = connectionTimeout;
 }
 
+void DcmSCU::setTcpPollInterval(const Sint32 pollInterval)
+{
+    m_tcpPollInterval = pollInterval;
+}
+
 void DcmSCU::setAssocConfigFileAndProfile(const OFString& filename, const OFString& profile)
 {
     m_assocConfigFilename = filename;
@@ -2650,6 +2671,11 @@ Sint32 DcmSCU::getConnectionTimeout() const
     return m_tcpConnectTimeout;
 }
 
+Sint32 DcmSCU::getTcpPollInterval() const
+{
+    return m_tcpPollInterval;
+}
+
 OFString DcmSCU::getStorageDir() const
 {
     return m_storageDir;

dcmnet/tests/tests.cc

@@ -51,6 +51,9 @@ OFTEST_REGISTER(dcmnet_scu_sendNSETRequest_fails_when_requestedsopinstance_is_em
 OFTEST_REGISTER(dcmnet_scu_sendNSETRequest_succeeds_and_modifies_instance_when_scp_has_instance);
 OFTEST_REGISTER(dcmnet_scu_sendNSETRequest_succeeds_and_sets_responsestatuscode_from_scp_when_scp_sets_error_status);
 
+OFTEST_REGISTER(dcmnet_scu_negotiateAssociation_fails_when_token_is_initially_cancelled);
+OFTEST_REGISTER(dcmnet_scu_negotiateAssociation_fails_when_token_gets_cancelled_after_3_calls);
+OFTEST_REGISTER(dcmnet_scu_negotiateAssociation_fails_when_connectionTimeout_elapses);
 #endif // WITH_THREADS
 
 OFTEST_MAIN("dcmnet")

dcmnet/tests/tscuscp.cc

@@ -505,6 +505,118 @@ OFTEST(dcmnet_scu_getConectionTimeout_returns_scu_tcp_connection_timeout)
     OFCHECK(scu.getConnectionTimeout() == 42);
 }
 
+struct CancelToken : IDcmCancelToken
+{
+    CancelToken(Uint32 cancelAfterNumCalls)
+        : m_canceled(false)
+        , m_cancelAfterNumCalls(cancelAfterNumCalls)
+        , m_callCount(0)
+    {}
+
+    bool IsCanceled() const /* override */
+    {
+        ++m_callCount;
+        if (m_callCount > m_cancelAfterNumCalls)
+            m_canceled = true;
+
+        return m_canceled;
+    }
+
+    mutable bool m_canceled;
+    const Uint32 m_cancelAfterNumCalls;
+    mutable Uint32 m_callCount;
+};
+
+
+struct TcpCancelFixture
+{
+    TcpCancelFixture()
+    {
+        m_scu.setPeerAETitle("ACCEPTOR");
+        m_scu.setAETitle("REQUESTOR");
+        m_scu.setPeerHostName("localhost");
+        m_scu.setPeerPort(GetUnusedPort());
+
+        OFList<OFString> ts;
+        ts.push_back(UID_LittleEndianImplicitTransferSyntax);
+        OFCHECK(m_scu.addPresentationContext(UID_VerificationSOPClass, ts, ASC_SC_ROLE_DEFAULT).good());
+    }
+
+    // Temporarily start an SCP to get an unused port
+    // Note: The port is not reserved after this call returns
+    static Uint16 GetUnusedPort()
+    {
+        TestSCP m_scp;
+        DcmSCPConfig& config = m_scp.getConfig();
+        configure_scp_for_echo(config, 0, ASC_SC_ROLE_SCP);
+        config.setAETitle("ACCEPTOR");
+        config.setConnectionBlockingMode(DUL_NOBLOCK);
+        config.setConnectionTimeout(4);
+        OFCHECK(m_scp.openListenPort().good());
+        const Uint16 port = config.getPort();
+        m_scp.join();
+        return port;
+    }
+
+    DcmSCU m_scu;
+
+};
+
+OFTEST(dcmnet_scu_negotiateAssociation_fails_when_token_is_initially_cancelled)
+{
+    TcpCancelFixture m_fixture;
+    DcmSCU& scu = m_fixture.m_scu;
+
+    scu.setTcpPollInterval(1);
+    scu.setConnectionTimeout(60);
+
+    OFCHECK(scu.initNetwork().good());
+
+    CancelToken tok(0);
+    tok.m_canceled = true;
+
+    const OFCondition result = scu.negotiateAssociation(&tok);
+
+    OFCHECK(result.code() == DULC_TCPINITERROR);
+    OFCHECK(tok.m_callCount == 1);
+}
+
+OFTEST(dcmnet_scu_negotiateAssociation_fails_when_token_gets_cancelled_after_3_calls)
+{
+    TcpCancelFixture m_fixture;
+    DcmSCU& scu = m_fixture.m_scu;
+
+    scu.setTcpPollInterval(1);
+    scu.setConnectionTimeout(60);
+
+    OFCHECK(scu.initNetwork().good());
+
+    CancelToken tok(3);
+
+    const OFCondition result = scu.negotiateAssociation(&tok);
+
+    OFCHECK(result.code() == DULC_TCPINITERROR);
+    OFCHECK(tok.m_callCount == 4);
+}
+
+OFTEST(dcmnet_scu_negotiateAssociation_fails_when_connectionTimeout_elapses)
+{
+
+    TcpCancelFixture m_fixture;
+    DcmSCU& scu = m_fixture.m_scu;
+
+    scu.setTcpPollInterval(1);
+    scu.setConnectionTimeout(5);
+
+    CancelToken tok(15); // Should never be called 15 times since connection timeout is reached first
+
+    OFCHECK(scu.initNetwork().good());
+    const OFCondition result = scu.negotiateAssociation(&tok);
+
+    OFCHECK(result.code() == DULC_TCPINITERROR);
+    OFCHECK(!tok.m_canceled);
+}
+
 
 /** Helper function for testing role selection, test "dcmnet_scp_role_selection".
  *  @param  r_req The role selection setting from the association requestor