Skip to content

Commit 44d106f

Browse files
ethourisMikolaj MaleckiClément Gérouville
authored
[BUG] Incorrect error code when srt_listen on closed socket (#3298)
* [BUG] Incorrect error code when srt_listen on closed socket * Fixed API description * Ask Codespell to ignore the TEST() macro. * Replace few if statements by a switch statement. * Status SRTS_NONEXIST throws MN_CLOSED instead of MN_ISUNBOUND * Manage the rendez-vous case first --------- Co-authored-by: Mikolaj Malecki <mmalecki@haivision.com> Co-authored-by: Clément Gérouville <cgerouville@haivision.com>
1 parent f04c00c commit 44d106f

4 files changed

Lines changed: 54 additions & 18 deletions

File tree

docs/API/API-functions.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -612,9 +612,10 @@ the listener socket to accept group connections
612612
| [`SRT_EINVPARAM`](#srt_einvparam) | Value of `backlog` is 0 or negative. |
613613
| [`SRT_EINVSOCK`](#srt_einvsock) | Socket [`u`](#u) indicates no valid SRT socket. |
614614
| [`SRT_EUNBOUNDSOCK`](#srt_eunboundsock) | [`srt_bind`](#srt_bind) has not yet been called on that socket. |
615+
| [`SRT_ESCLOSED`](#srt_esclosed) | The socket has been closed |
615616
| [`SRT_ERDVNOSERV`](#srt_erdvnoserv) | [`SRTO_RENDEZVOUS`](API-socket-options.md#SRTO_RENDEZVOUS) flag is set to true on specified socket. |
616617
| [`SRT_EINVOP`](#srt_einvop) | Internal error (should not happen when [`SRT_EUNBOUNDSOCK`](#srt_eunboundsock) is reported). |
617-
| [`SRT_ECONNSOCK`](#srt_econnsock) | The socket is already connected. |
618+
| [`SRT_ECONNSOCK`](#srt_econnsock) | The socket is currently being used to establish a connection (like by `srt_connect`) |
618619
| [`SRT_EDUPLISTEN`](#srt_eduplisten) | The address used in [`srt_bind`](#srt_bind) by this socket is already occupied by another listening socket. <br/> Binding multiple sockets to one IP address and port is allowed, as long as <br/> [`SRTO_REUSEADDR`](API-socket-options.md#SRTO_REUSEADDRS) is set to true, but only one of these sockets can be set up as a listener. |
619620
| <img width=240px height=1px/> | <img width=710px height=1px/> |
620621

scripts/codespell/codespell.cfg

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ builtin = clear,rare,informal,code
66

77
# Ignore words listed in this file.
88
ignore-words = ./scripts/codespell/codespell_whitelist.txt
9+
ignore-regex = TEST\(.*\)
910

1011
# Add custom dictionary file.
1112
dictionary = ./scripts/codespell/codespell_dictionary.txt,-
1213

1314
# Skip checking files or directories.
1415
skip = ./build/*,./.git/*
16+

srtcore/api.cpp

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,27 +1144,34 @@ int srt::CUDTUnited::listen(const SRTSOCKET u, int backlog)
11441144
// it could have changed the state. It could be also set listen in another
11451145
// thread, so check it out.
11461146

1147-
// do nothing if the socket is already listening
1148-
if (s->m_Status == SRTS_LISTENING)
1149-
return 0;
1150-
1151-
// a socket can listen only if is in OPENED status
1152-
if (s->m_Status != SRTS_OPENED)
1153-
throw CUDTException(MJ_NOTSUP, MN_ISUNBOUND, 0);
1154-
1155-
// [[using assert(s->m_Status == OPENED)]];
1156-
1157-
// listen is not supported in rendezvous connection setup
11581147
if (s->core().m_config.bRendezvous)
11591148
throw CUDTException(MJ_NOTSUP, MN_ISRENDEZVOUS, 0);
11601149

1161-
s->m_uiBackLog = backlog;
1162-
1163-
// [[using assert(s->m_Status == OPENED)]]; // (still, unchanged)
1150+
switch(s->m_Status)
1151+
{
1152+
case SRTS_INIT:
1153+
throw CUDTException(MJ_NOTSUP, MN_ISUNBOUND, 0);
1154+
break;
1155+
case SRTS_OPENED:
1156+
s->m_uiBackLog = backlog;
1157+
s->core().setListenState(); // propagates CUDTException,
1158+
s->m_Status = SRTS_LISTENING;
1159+
break;
1160+
case SRTS_LISTENING:
1161+
s->m_uiBackLog = backlog;
1162+
break;
1163+
case SRTS_CONNECTING:
1164+
case SRTS_CONNECTED:
1165+
throw CUDTException(MJ_NOTSUP, MN_ISCONNECTED, 0);
1166+
break;
1167+
case SRTS_BROKEN:
1168+
case SRTS_CLOSING:
1169+
case SRTS_CLOSED:
1170+
case SRTS_NONEXIST:
1171+
throw CUDTException(MJ_SETUP, MN_CLOSED, 0);
1172+
break;
1173+
}
11641174

1165-
s->core().setListenState(); // propagates CUDTException,
1166-
// if thrown, remains in OPENED state if so.
1167-
s->m_Status = SRTS_LISTENING;
11681175

11691176
return 0;
11701177
}

test/test_connection_timeout.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,4 +390,30 @@ TEST(TestConnectionAPI, Accept)
390390
srt_cleanup();
391391
}
392392

393+
TEST(TestConnectionAPI, Listen)
394+
{
395+
using namespace std::chrono;
396+
using namespace srt;
397+
srt_startup();
398+
399+
SRTSOCKET s = srt_create_socket();
400+
int listen_stat1, listen_stat2, listen_stat3;
401+
402+
sockaddr_any sa = srt::CreateAddr("localhost", 5555, AF_INET);
403+
404+
ASSERT_NE(srt_bind(s, sa.get(), sa.size()), -1);
405+
listen_stat1 = srt_listen(s, 1);
406+
listen_stat2 = srt_listen(s, 5);
407+
srt_close(s);
408+
listen_stat3 = srt_listen(s, 5);
409+
410+
int err = srt_getlasterror(NULL);
411+
std::cout << "Listen after close error: " << srt_strerror(err, 0) << std::endl;
412+
413+
EXPECT_EQ(listen_stat1, 0);
414+
EXPECT_EQ(listen_stat2, 0);
415+
EXPECT_EQ(listen_stat3, -1);
416+
417+
srt_cleanup();
418+
}
393419

0 commit comments

Comments
 (0)