Skip to content

Commit f2b736b

Browse files
MysticialMysticial
committed
Reduce scope of socket lock on send.
1 parent 106244b commit f2b736b

File tree

4 files changed

+64
-52
lines changed

4 files changed

+64
-52
lines changed

‎Common/Cpp/Sockets/ClientSocket_POSIX.h‎

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@ namespace PokemonAutomation{
2727
class ClientSocket_POSIX final : public AbstractClientSocket{
2828
public:
2929
ClientSocket_POSIX()
30-
: m_socket(-1)
31-
{}
30+
: m_socket(socket(AF_INET, SOCK_STREAM, 0))
31+
{
32+
fcntl(m_socket, F_SETFL, O_NONBLOCK);
33+
}
3234

3335
virtual ~ClientSocket_POSIX(){
3436
close();
@@ -53,37 +55,28 @@ class ClientSocket_POSIX final : public AbstractClientSocket{
5355
return;
5456
}
5557
try{
56-
m_socket = socket(AF_INET, SOCK_STREAM, 0);
57-
fcntl(m_socket, F_SETFL, O_NONBLOCK);
5858
m_state.store(State::CONNECTING, std::memory_order_relaxed);
5959
m_thread = std::thread(
6060
&ClientSocket_POSIX::thread_loop,
6161
this, address, port
6262
);
6363
}catch (...){
64-
::close(m_socket);
65-
m_socket = -1;
6664
m_state.store(State::NOT_RUNNING, std::memory_order_relaxed);
6765
throw;
6866
}
6967
}
7068

7169

7270
virtual size_t blocking_send(const void* data, size_t bytes) override{
73-
std::unique_lock<std::mutex> lg(m_lock);
74-
constexpr int BLOCK_SIZE = (int)1 << 30;
75-
const char* ptr = (const char*)data;
76-
size_t sent = 0;
7771
if (m_socket == -1){
7872
return 0;
7973
}
80-
bool skip_wait = true;
81-
while (bytes > 0){
82-
if (!skip_wait){
83-
m_cv.wait_for(lg, std::chrono::milliseconds(1));
84-
}
85-
skip_wait = true;
8674

75+
constexpr int BLOCK_SIZE = (int)1 << 30;
76+
const char* ptr = (const char*)data;
77+
size_t sent = 0;
78+
79+
while (bytes > 0){
8780
size_t current = std::min<size_t>(bytes, BLOCK_SIZE);
8881
int current_sent = ::send(m_socket, ptr, (int)current, MSG_DONTWAIT);
8982
if (current_sent != -1){
@@ -96,6 +89,11 @@ class ClientSocket_POSIX final : public AbstractClientSocket{
9689
continue;
9790
}
9891

92+
std::unique_lock<std::mutex> lg(m_lock);
93+
if (state() == State::DESTRUCTING){
94+
break;
95+
}
96+
9997
int error = errno;
10098
// cout << "error = " << error << endl;
10199
switch (error){
@@ -106,6 +104,8 @@ class ClientSocket_POSIX final : public AbstractClientSocket{
106104
m_error = "POSIX Error Code: " + std::to_string(error);
107105
return sent;
108106
}
107+
108+
m_cv.wait_for(lg, std::chrono::milliseconds(1));
109109
}
110110
return sent;
111111
}
@@ -199,7 +199,15 @@ class ClientSocket_POSIX final : public AbstractClientSocket{
199199

200200
if (bytes > 0){
201201
m_listeners.run_method_unique(&Listener::on_receive_data, buffer, bytes);
202-
}else if (bytes < 0){
202+
continue;
203+
}
204+
205+
std::unique_lock<std::mutex> lg(m_lock);
206+
if (state() == State::DESTRUCTING){
207+
return;
208+
}
209+
210+
if (bytes < 0){
203211
// cout << "error = " << error << endl;
204212
switch (error){
205213
case EAGAIN:
@@ -212,15 +220,14 @@ class ClientSocket_POSIX final : public AbstractClientSocket{
212220
}
213221
}
214222

215-
std::unique_lock<std::mutex> lg(m_lock);
216223
m_cv.wait_for(lg, std::chrono::milliseconds(1));
217224
}
218225

219226
}
220227

221228

222229
private:
223-
int m_socket;
230+
const int m_socket;
224231

225232
std::string m_error;
226233

‎Common/Cpp/Sockets/ClientSocket_WinSocket.h‎

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,15 @@ namespace PokemonAutomation{
2323
class ClientSocket_WinSocket final : public AbstractClientSocket{
2424
public:
2525
ClientSocket_WinSocket()
26-
: m_socket(INVALID_SOCKET)
27-
{}
26+
: m_socket(::socket(AF_INET, SOCK_STREAM, 0))
27+
{
28+
u_long non_blocking = 1;
29+
if (ioctlsocket(m_socket, FIONBIO, &non_blocking)){
30+
// cout << "ioctlsocket() Failed" << endl;
31+
closesocket(m_socket);
32+
return;
33+
}
34+
}
2835

2936
virtual ~ClientSocket_WinSocket(){
3037
close();
@@ -49,41 +56,28 @@ class ClientSocket_WinSocket final : public AbstractClientSocket{
4956
return;
5057
}
5158
try{
52-
m_socket = socket(AF_INET, SOCK_STREAM, 0);
53-
u_long non_blocking = 1;
54-
if (ioctlsocket(m_socket, FIONBIO, &non_blocking)){
55-
// cout << "ioctlsocket() Failed" << endl;
56-
return;
57-
}
5859
m_state.store(State::CONNECTING, std::memory_order_relaxed);
5960
m_thread = std::thread(
6061
&ClientSocket_WinSocket::thread_loop,
6162
this, address, port
6263
);
6364
}catch (...){
64-
closesocket(m_socket);
65-
m_socket = INVALID_SOCKET;
6665
m_state.store(State::NOT_RUNNING, std::memory_order_relaxed);
6766
throw;
6867
}
6968
}
7069

7170

7271
virtual size_t blocking_send(const void* data, size_t bytes) override{
73-
std::unique_lock<std::mutex> lg(m_lock);
74-
constexpr int BLOCK_SIZE = (int)1 << 30;
75-
const char* ptr = (const char*)data;
76-
size_t sent = 0;
7772
if (m_socket == INVALID_SOCKET){
7873
return 0;
7974
}
80-
bool skip_wait = true;
81-
while (bytes > 0){
82-
if (!skip_wait){
83-
m_cv.wait_for(lg, std::chrono::milliseconds(1));
84-
}
85-
skip_wait = true;
8675

76+
constexpr int BLOCK_SIZE = (int)1 << 30;
77+
const char* ptr = (const char*)data;
78+
size_t sent = 0;
79+
80+
while (bytes > 0 && state() == State::CONNECTED){
8781
size_t current = std::min<size_t>(bytes, BLOCK_SIZE);
8882
int current_sent = ::send(m_socket, ptr, (int)current, 0);
8983
if (current_sent != SOCKET_ERROR){
@@ -98,13 +92,21 @@ class ClientSocket_WinSocket final : public AbstractClientSocket{
9892

9993
int error = WSAGetLastError();
10094
// cout << "error = " << error << endl;
95+
96+
std::unique_lock<std::mutex> lg(m_lock);
97+
if (state() == State::DESTRUCTING){
98+
break;
99+
}
100+
101101
switch (error){
102102
case WSAEWOULDBLOCK:
103103
break;
104104
default:
105105
m_error = "WSA Error Code: " + std::to_string(error);
106106
return sent;
107107
}
108+
109+
m_cv.wait_for(lg, std::chrono::milliseconds(1));
108110
}
109111
return sent;
110112
}
@@ -184,7 +186,6 @@ class ClientSocket_WinSocket final : public AbstractClientSocket{
184186
int bytes;
185187
int error = 0;
186188
{
187-
std::unique_lock<std::mutex> lg(m_lock);
188189
State state = m_state.load(std::memory_order_relaxed);
189190
if (state == State::DESTRUCTING){
190191
return;
@@ -197,27 +198,33 @@ class ClientSocket_WinSocket final : public AbstractClientSocket{
197198

198199
if (bytes > 0){
199200
m_listeners.run_method_unique(&Listener::on_receive_data, buffer, bytes);
200-
}else if (bytes == SOCKET_ERROR){
201+
continue;
202+
}
203+
204+
std::unique_lock<std::mutex> lg(m_lock);
205+
if (state() == State::DESTRUCTING){
206+
return;
207+
}
208+
209+
if (bytes == SOCKET_ERROR){
201210
// cout << "error = " << error << endl;
202211
switch (error){
203212
case WSAEWOULDBLOCK:
204213
break;
205214
default:
206-
std::unique_lock<std::mutex> lg(m_lock);
207215
m_error = "WSA Error Code: " + std::to_string(error);
208216
return;
209217
}
210218
}
211219

212-
std::unique_lock<std::mutex> lg(m_lock);
213220
m_cv.wait_for(lg, std::chrono::milliseconds(1));
214221
}
215222

216223
}
217224

218225

219226
private:
220-
SOCKET m_socket;
227+
const SOCKET m_socket;
221228

222229
std::string m_error;
223230

‎SerialPrograms/Source/NintendoSwitch/Controllers/SysbotBase/SysbotBase_Connection.cpp‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,7 @@ TcpSysbotBase_Connection::TcpSysbotBase_Connection(
7979

8080
TcpSysbotBase_Connection::~TcpSysbotBase_Connection(){
8181
try{
82-
std::string str = "detachController\n";
83-
m_socket.blocking_send(str.data(), str.size());
82+
write_data("detachController\n");
8483
}catch (...){}
8584
m_socket.remove_listener(*this);
8685
m_socket.close();
@@ -103,6 +102,7 @@ std::map<ControllerType, std::set<ControllerFeature>> TcpSysbotBase_Connection::
103102
}
104103

105104
void TcpSysbotBase_Connection::write_data(const std::string& data){
105+
WriteSpinLock lg(m_send_lock);
106106
m_socket.blocking_send(data.data(), data.size());
107107
}
108108

@@ -167,12 +167,9 @@ void TcpSysbotBase_Connection::on_connect_finished(const std::string& error_mess
167167

168168
m_logger.log(m_connecting_message + " (Success)", COLOR_BLUE);
169169

170-
{
171-
std::unique_lock<std::mutex> lg(m_lock);
172-
write_data("configure echoCommands 0\n");
173-
write_data("getVersion\n");
174-
write_data("configure mainLoopSleepTime 0\n");
175-
}
170+
write_data("configure echoCommands 0\n");
171+
write_data("getVersion\n");
172+
write_data("configure mainLoopSleepTime 0\n");
176173

177174
m_thread = std::thread(&TcpSysbotBase_Connection::thread_loop, this);
178175

‎SerialPrograms/Source/NintendoSwitch/Controllers/SysbotBase/SysbotBase_Connection.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class TcpSysbotBase_Connection : public ControllerConnection, private ClientSock
5050
std::string m_version;
5151
WallClock m_last_receive;
5252

53+
SpinLock m_send_lock;
5354
std::mutex m_lock;
5455
std::condition_variable m_cv;
5556
std::thread m_thread;

0 commit comments

Comments
 (0)