1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
|
From c79120c049d825fedeed70d5a1a9dc64d17ce9f0 Mon Sep 17 00:00:00 2001
From: Vasily Galkin <galkin-vv@ya.ru>
Date: Sun, 9 Feb 2020 23:27:26 +0300
Subject: [PATCH] Fix infinite loop on fast TCP disconnection
The commit a841b28 changed the condition for removing job from processing.
New flag MultiplexerJobStatus::continue_servicing become used
instead of checking pointer for NULL.
However for cases when TCPSocket::newJob() returns nullptr
the behaviour changed: earlier the job was removed, but after change
it is called again, since MultiplexerJobStatus equal to {true, nullptr}
means "run this job again".
This leads to problem with eating CPU and RAM on linux
https://github.com/debauchee/barrier/issues/470
There is similar windows problem, but not sure it is related.
https://github.com/debauchee/barrier/issues/552
Since it looks that the goal of a841b28 was only clarifying
object ownership and not changing job deletion behaviour,
this commit tries to get original behaviour and fix the bugs above
by returning {false, nullptr} instead of {true, nullptr}
when TCPSocket::newJob() returns nullptr.
---
src/lib/net/SecureSocket.cpp | 4 ++--
src/lib/net/TCPSocket.cpp | 25 +++++++++++++------------
src/lib/net/TCPSocket.h | 3 ++-
3 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/src/lib/net/SecureSocket.cpp b/src/lib/net/SecureSocket.cpp
index 99f626e8..92abea3c 100644
--- a/src/lib/net/SecureSocket.cpp
+++ b/src/lib/net/SecureSocket.cpp
@@ -761,7 +761,7 @@ MultiplexerJobStatus SecureSocket::serviceConnect(ISocketMultiplexerJob* job,
// If status > 0, success
if (status > 0) {
sendEvent(m_events->forIDataSocket().secureConnected());
- return {true, newJob()};
+ return newJobOrStopServicing();
}
// Retry case
@@ -793,7 +793,7 @@ MultiplexerJobStatus SecureSocket::serviceAccept(ISocketMultiplexerJob* job,
// If status > 0, success
if (status > 0) {
sendEvent(m_events->forClientListener().accepted());
- return {true, newJob()};
+ return newJobOrStopServicing();
}
// Retry case
diff --git a/src/lib/net/TCPSocket.cpp b/src/lib/net/TCPSocket.cpp
index 4f4251ad..09a8f17e 100644
--- a/src/lib/net/TCPSocket.cpp
+++ b/src/lib/net/TCPSocket.cpp
@@ -403,6 +403,15 @@ void TCPSocket::setJob(std::unique_ptr<ISocketMultiplexerJob>&& job)
}
}
+MultiplexerJobStatus TCPSocket::newJobOrStopServicing()
+{
+ auto new_job = newJob();
+ if (new_job)
+ return {true, std::move(new_job)};
+ else
+ return {false, {}};
+}
+
std::unique_ptr<ISocketMultiplexerJob> TCPSocket::newJob()
{
// note -- must have m_mutex locked on entry
@@ -519,22 +528,14 @@ MultiplexerJobStatus TCPSocket::serviceConnecting(ISocketMultiplexerJob* job, bo
catch (XArchNetwork& e) {
sendConnectionFailedEvent(e.what());
onDisconnected();
- auto new_job = newJob();
- if (new_job)
- return {true, std::move(new_job)};
- else
- return {false, {}};
+ return newJobOrStopServicing();
}
}
if (write) {
sendEvent(m_events->forIDataSocket().connected());
onConnected();
- auto new_job = newJob();
- if (new_job)
- return {true, std::move(new_job)};
- else
- return {false, {}};
+ return newJobOrStopServicing();
}
return {true, {}};
@@ -548,7 +549,7 @@ MultiplexerJobStatus TCPSocket::serviceConnected(ISocketMultiplexerJob* job,
if (error) {
sendEvent(m_events->forISocket().disconnected());
onDisconnected();
- return {true, newJob()};
+ return newJobOrStopServicing();
}
EJobResult writeResult = kRetry;
@@ -603,7 +604,7 @@ MultiplexerJobStatus TCPSocket::serviceConnected(ISocketMultiplexerJob* job,
if (writeResult == kBreak || readResult == kBreak) {
return {false, {}};
} else if (writeResult == kNew || readResult == kNew) {
- return {true, newJob()};
+ return newJobOrStopServicing();
} else {
return {true, {}};
}
diff --git a/src/lib/net/TCPSocket.h b/src/lib/net/TCPSocket.h
index 28891353..0b988886 100644
--- a/src/lib/net/TCPSocket.h
+++ b/src/lib/net/TCPSocket.h
@@ -76,7 +76,8 @@ protected:
void removeJob();
void setJob(std::unique_ptr<ISocketMultiplexerJob>&& job);
-
+ MultiplexerJobStatus newJobOrStopServicing();
+
bool isReadable() { return m_readable; }
bool isWritable() { return m_writable; }
--
2.24.1
|
GitWeb