[core] Changed SRTO_CONNTIMEO restriction to PRE (#1864)

* Changed SRTO_CONNTIMEO to be pre-restricted * Added timeout check to TestConnectionTimeout.BlockingLoop with 200ms tolerance. GitHub CI (macOS) gave up to 141 ms extra delay. Co-authored-by: Maxim Sharabayko <maxsharabayko@haivision.com>
Haivision · Mar 18, 2021 · b38bb7f · b38bb7f
1 parent 00e42d7
commit b38bb7f
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 8 deletions.
diff --git a/docs/APISocketOptions.md b/docs/APISocketOptions.md
@@ -202,7 +202,7 @@ The following table lists SRT socket options in alphabetical order. Option detai
 | :----------------------------------------------------- | :---: | :------: | :-------: | :-----: | :-----------: | :------: |:---:|:-----:|
 | [`SRTO_BINDTODEVICE`](#SRTO_BINDTODEVICE)              | 1.4.2 | pre-bind | `string`  |         |               |          | RW  | GSD+  |
 | [`SRTO_CONGESTION`](#SRTO_CONGESTION)                  | 1.3.0 | pre      | `string`  |         | "live"        | *        | W   | S     |
-| [`SRTO_CONNTIMEO`](#SRTO_CONNTIMEO)                    | 1.1.2 | post     | `int32_t` | ms      | 3000          | 0..      | W   | GSD+  |
+| [`SRTO_CONNTIMEO`](#SRTO_CONNTIMEO)                    | 1.1.2 | pre      | `int32_t` | ms      | 3000          | 0..      | W   | GSD+  |
 | [`SRTO_DRIFTTRACER`](#SRTO_DRIFTTRACER)                | 1.4.2 | post     | `bool`    |         | true          |          | RW  | GSD   |
 | [`SRTO_ENFORCEDENCRYPTION`](#SRTO_ENFORCEDENCRYPTION)  | 1.3.2 | pre      | `bool`    |         | true          |          | W   | GSD   |
 | [`SRTO_EVENT`](#SRTO_EVENT)                            |       |          | `int32_t` | flags   |               |          | R   | S     |
@@ -311,7 +311,7 @@ rather change the whole set of options using the [`SRTO_TRANSTYPE`](#SRTO_TRANST
 
 | OptName            | Since | Restrict |   Type    | Units  | Default  | Range  | Dir | Entity |
 | ------------------ | ----- | -------- | --------- | ------ | -------- | ------ | --- | ------ |
-| `SRTO_CONNTIMEO`   | 1.1.2 | post     | `int32_t` | msec   | 3000     | 0..    | W   | GSD+   |
+| `SRTO_CONNTIMEO`   | 1.1.2 | pre      | `int32_t` | msec   | 3000     | 0..    | W   | GSD+   |
 
 Connect timeout. This option applies to the caller and rendezvous connection
 modes. For the rendezvous mode (see `SRTO_RENDEZVOUS`) the effective connection timeout

diff --git a/srtcore/core.cpp b/srtcore/core.cpp
@@ -125,7 +125,6 @@ extern const SRT_SOCKOPT srt_post_opt_list [SRT_SOCKOPT_NPOST] = {
     SRTO_MININPUTBW,
     SRTO_OHEADBW,
     SRTO_SNDDROPDELAY,
-    SRTO_CONNTIMEO,
     SRTO_DRIFTTRACER,
     SRTO_LOSSMAXTTL
 };
@@ -172,6 +171,7 @@ struct SrtOptionAction
         flags[SRTO_SNDDROPDELAY]       = SRTO_R_PRE;
         flags[SRTO_NAKREPORT]          = SRTO_R_PRE;
         flags[SRTO_VERSION]            = SRTO_R_PRE;
+        flags[SRTO_CONNTIMEO]          = SRTO_R_PRE;
         flags[SRTO_LOSSMAXTTL]         = 0 | SRTO_POST_SPEC;
         flags[SRTO_RCVLATENCY]         = SRTO_R_PRE;
         flags[SRTO_PEERLATENCY]        = SRTO_R_PRE;

diff --git a/srtcore/core.h b/srtcore/core.h
@@ -116,7 +116,7 @@ enum AckDataItem
 };
 const size_t ACKD_FIELD_SIZE = sizeof(int32_t);
 
-static const size_t SRT_SOCKOPT_NPOST = 13;
+static const size_t SRT_SOCKOPT_NPOST = 12;
 extern const SRT_SOCKOPT srt_post_opt_list [];
 
 enum GroupDataItem

diff --git a/test/test_connection_timeout.cpp b/test/test_connection_timeout.cpp
@@ -126,7 +126,6 @@ TEST_F(TestConnectionTimeout, Nonblocking) {
         int wlen = 2;
         SRTSOCKET write[2];
 
-        using namespace std;
         const chrono::steady_clock::time_point chrono_ts_start = chrono::steady_clock::now();
 
         // Here we check the connection timeout.
@@ -143,9 +142,8 @@ TEST_F(TestConnectionTimeout, Nonblocking) {
         const chrono::steady_clock::time_point chrono_ts_end = chrono::steady_clock::now();
         const auto delta_ms = chrono::duration_cast<chrono::milliseconds>(chrono_ts_end - chrono_ts_start).count();
         // Confidence interval border : +/-80 ms
-        EXPECT_LE(delta_ms, connection_timeout_ms + 80);
-        EXPECT_GE(delta_ms, connection_timeout_ms - 80);
-        cerr << "Timeout was: " << delta_ms << "\n";
+        EXPECT_LE(delta_ms, connection_timeout_ms + 80) << "Timeout was: " << delta_ms;
+        EXPECT_GE(delta_ms, connection_timeout_ms - 80) << "Timeout was: " << delta_ms;
 
         EXPECT_EQ(rlen, 1);
         EXPECT_EQ(read[0], client_sock);
@@ -186,8 +184,14 @@ TEST_F(TestConnectionTimeout, BlockingLoop)
     const sockaddr* psa = reinterpret_cast<const sockaddr*>(&m_sa);
     for (int i = 0; i < 10; ++i)
     {
+        const chrono::steady_clock::time_point chrono_ts_start = chrono::steady_clock::now();
         EXPECT_EQ(srt_connect(client_sock, psa, sizeof m_sa), SRT_ERROR);
 
+        const auto delta_ms = chrono::duration_cast<chrono::milliseconds>(chrono::steady_clock::now() - chrono_ts_start).count();
+        // Confidence interval border : +/-200 ms
+        EXPECT_LE(delta_ms, connection_timeout_ms + 200) << "Timeout was: " << delta_ms;
+        EXPECT_GE(delta_ms, connection_timeout_ms - 200) << "Timeout was: " << delta_ms;
+
         const int error_code = srt_getlasterror(nullptr);
         EXPECT_EQ(error_code, SRT_ENOSERVER);
         if (error_code != SRT_ENOSERVER)