Skip to content

Commit 70aa26b

Browse files
kendyMinion3665
authored andcommitted
Make sure we don't enter wait()/wait_until() with anything to consume
Perform the last check for any events before we start waiting. It must be done with theMutex taken, so that we don't miss anything. It is not a problem to return a bit too eagerly (when there is data to read or accept), but OTOH we should try to avoid a potential busy loop after EOF. This is a follow-up of the iOS interactivity improvement (with change id Ic7295393be8b5aa76cfd56fe579b39032b4a10ca). Signed-off-by: Jan Holesovsky <kendy@collabora.com> Change-Id: Idc3304c2ecb44c3557aa3edb928f82037c7e4e19
1 parent 5a6b9a3 commit 70aa26b

File tree

1 file changed

+62
-3
lines changed

1 file changed

+62
-3
lines changed

net/FakeSocket.cpp

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ struct FakeSocketPair
5757
int connectingFd;
5858
bool shutdown[2];
5959
bool readable[2];
60+
bool eofDetected[2]; ///< Flag for fakeSocketHasAnyPendingActivityGlobal() to avoid a busy loop after EOF.
6061
std::vector<std::vector<char>> buffer[2];
6162

6263
FakeSocketPair()
@@ -69,6 +70,8 @@ struct FakeSocketPair
6970
shutdown[1] = false;
7071
readable[0] = false;
7172
readable[1] = false;
73+
eofDetected[0] = false;
74+
eofDetected[1] = false;
7275
}
7376
};
7477

@@ -282,19 +285,75 @@ static bool checkForPoll(struct pollfd *pollfds, int nfds)
282285
return retval;
283286
}
284287

288+
/**
289+
* Scan all the 'fds' we have for fakeSockets, and check if any of them
290+
* reports any activity, so that we should rather skip the wait()/wait_until()
291+
* in fakeSocketWaitAny().
292+
*
293+
* NB: Must be called with theMutex held.
294+
*/
295+
static bool fakeSocketHasAnyPendingActivityGlobal()
296+
{
297+
for (const auto& pairPtr : fds)
298+
{
299+
if (!pairPtr)
300+
continue;
301+
302+
FakeSocketPair& p = *pairPtr;
303+
304+
// Endpoint 0 (even fd) and endpoint 1 (odd fd)
305+
for (int K = 0; K < 2; ++K)
306+
{
307+
// Endpoint closed?
308+
if (p.fd[K] == -1)
309+
continue;
310+
311+
const int N = 1 - K;
312+
313+
// Pending accept on a listening socket?
314+
// Only meaningful on K==0
315+
if (K == 0 && p.listening && p.connectingFd != -1)
316+
return true;
317+
318+
// Buffered data ready for this endpoint?
319+
if (!p.buffer[K].empty())
320+
return true;
321+
322+
// Peer shutdown/closed?
323+
// Detect EOF via "readable[K]" so a read() can consume it (0 bytes).
324+
// But in fakeSocketRead(), we explicitly stay readable after peer
325+
// closed or shut down, so use a flag to return 'true' just once.
326+
if ((p.shutdown[N] || p.fd[N] == -1) && p.readable[K] && !p.eofDetected[K])
327+
{
328+
p.eofDetected[K] = true; // mark EOF consumed
329+
return true;
330+
}
331+
}
332+
}
333+
return false;
334+
}
335+
285336
/**
286337
* Wait for any event on any of the fake sockets (theCV is notified on write/close/connect/etc.)
287338
*/
288339
void fakeSocketWaitAny(int timeoutUs)
289340
{
341+
if (timeoutUs == 0)
342+
return;
343+
290344
std::unique_lock<std::mutex> lock(theMutex);
345+
346+
// This check must be run under theMutex taken, so that we
347+
// don't enter wait()/wait_until() if we've got new events
348+
// that we might have missed since the last poll()
349+
if (fakeSocketHasAnyPendingActivityGlobal())
350+
return;
351+
291352
if (timeoutUs < 0)
292353
{
293354
theCV.wait(lock);
294355
return;
295356
}
296-
if (timeoutUs == 0)
297-
return;
298357

299358
auto const deadline = std::chrono::steady_clock::now() + std::chrono::microseconds(timeoutUs);
300359
theCV.wait_until(lock, deadline);
@@ -673,7 +732,7 @@ int fakeSocketShutdown(int fd)
673732
}
674733

675734
pair.shutdown[K] = true;
676-
pair.readable[K] = true;
735+
pair.readable[N] = true; // wake the peer to observe EOF
677736

678737
theCV.notify_all();
679738

0 commit comments

Comments
 (0)