Skip to content

Commit aaede46

Browse files
committed
[example] Fix a few nitpicks
Lets clarify a few things better, and fix a couple of bugs: - In one example a member wasn't initialized in the constructor. - In another a task didn't set deadline when waiting for a signal. It led to a busy loop. Follow up #21
1 parent 69206c4 commit aaede46

File tree

3 files changed

+41
-10
lines changed

3 files changed

+41
-10
lines changed

examples/iocore_01_tcp_hello/main.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ class MyClient final
3333
mySock->Open({});
3434
mg::aio::TCPSocketConnectParams connParams;
3535
connParams.myEndpoint = mg::net::HostMakeLocalIPV4(aPort).ToString();
36+
37+
// From PostConnect() moment the socket runs in IOCore and its event-callbacks
38+
// will start firing.
3639
mySock->PostConnect(connParams, this);
3740
}
3841

@@ -58,6 +61,7 @@ class MyClient final
5861
{
5962
MG_LOG_INFO("Client.OnRecv", "%d: got response", myID);
6063
MG_BOX_ASSERT(aStream.GetReadSize() > 0);
64+
// Closure is asynchronous as well. You get OnClose() event when it is complete.
6165
mySock->PostClose();
6266
}
6367

@@ -74,6 +78,11 @@ class MyClient final
7478
void
7579
OnClose() final
7680
{
81+
// Here the socket should be deleted, ideally. But also you can reopen it and
82+
// reuse. The socket will stay valid anyway. Moreover, the socket still can be
83+
// used for deadlines and wakeups, even when it is closed. This allows to
84+
// implement automatic reconnect with a fallback period like 1 second or so.
85+
7786
MG_LOG_INFO("Client.OnClose", "%d", myID);
7887
// Try to always have all deletions in a single place. And the only suitable place
7988
// is on-close. This is the only place where you can safely assume that the socket
@@ -185,6 +194,8 @@ class MyServer final
185194
Start()
186195
{
187196
mg::box::Error::Ptr err;
197+
// From Listen() moment the server's socket runs in IOCore and its event-callbacks
198+
// start firing.
188199
bool ok = myServer->Listen(mg::net::SocketMaxBacklog(), this, err);
189200
MG_BOX_ASSERT(ok);
190201
}
@@ -209,6 +220,8 @@ class MyServer final
209220
void
210221
OnClose() final
211222
{
223+
// The server will not run anymore after this event. It is still valid, but won't
224+
// fire any events.
212225
MG_LOG_INFO("Server.OnClose", "");
213226
}
214227

examples/iocore_03_pipeline/main.cpp

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ class CalcClient final
7777
std::function<void(int64_t)>&& aOnComplete)
7878
{
7979
MG_LOG_INFO("Client.Submit", "new request");
80+
// This function can be called by any thread in our code. Not only by IOCore
81+
// worker serving this client's socket right now. Need to be careful with thread
82+
// safety here.
83+
//
84+
// The solution we can do is have a thread-safe queue of requests submitted to the
85+
// client. When the queue gets populated, we wakeup the client to pop the queue
86+
// and handle the requests.
8087
CalcRequest* req = new CalcRequest();
8188
req->myOp = aOp;
8289
req->myArg1 = aArg1;
@@ -119,12 +126,14 @@ class CalcClient final
119126
{
120127
CalcRequest* tail;
121128
CalcRequest* head = myFrontQueue.PopAll(tail);
129+
// Check if spurious wakeup.
122130
if (head == nullptr)
123131
return;
124132

125-
// Save the requests for later response handling.
133+
// Move the requests from the thread-safe front queue to the local plain simple
134+
// list. For later response handling.
126135
myQueue.Append(head, tail);
127-
// Send requests in bulk.
136+
// Send requests in bulk, in one stream of bytes.
128137
mg::net::BufferStream data;
129138
while (head != nullptr)
130139
{
@@ -181,9 +190,14 @@ class CalcClient final
181190
}
182191

183192
mg::aio::TCPSocket* mySock;
184-
// Queue of sent requests waiting for their responses.
193+
194+
// Queue of sent requests waiting for their responses. It is not thread-safe, just a
195+
// normal list. But it is only accessed inside client's socket context. Hence never is
196+
// touched by more than one thread. No need for a mutex.
185197
mg::box::ForwardList<CalcRequest> myQueue;
186-
// Queue if requests submitted by other threads. Not yet sent to the network.
198+
199+
// Queue if requests submitted by other threads. Not yet sent to the network. It is
200+
// thread-safe.
187201
mg::box::MultiProducerQueueIntrusive<CalcRequest> myFrontQueue;
188202
};
189203

@@ -360,7 +374,8 @@ class MyRequest
360374
aSelf->myTask.PostSignal();
361375
});
362376
// Wait for the signal in a yielding loop, to handle potential spurious wakeups.
363-
while (!co_await aSelf->myTask.AsyncReceiveSignal());
377+
while (!co_await aSelf->myTask.AsyncReceiveSignal())
378+
aSelf->myTask.SetWait();
364379
//
365380
MG_LOG_INFO("MyRequest.Execute", "%d: got response %lld", myID, (long long)res);
366381

@@ -371,7 +386,8 @@ class MyRequest
371386
res = aRes;
372387
aSelf->myTask.PostSignal();
373388
});
374-
while (!co_await aSelf->myTask.AsyncReceiveSignal());
389+
while (!co_await aSelf->myTask.AsyncReceiveSignal())
390+
aSelf->myTask.SetWait();
375391
MG_LOG_INFO("MyRequest.Execute", "%d: got response %lld", myID, (long long)res);
376392

377393
// 'delete this' + co_return wouldn't work here. Because deletion of the self

examples/iocore_04_tcp_periodic/main.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ class MyPeer final
135135
mg::net::Socket aSock)
136136
: myID(aID)
137137
, mySock(new mg::aio::TCPSocket(aCore))
138+
, mySendSize(0)
138139
, mySendDeadline(0)
139140
{
140141
mySock->Open({});
@@ -203,11 +204,12 @@ class MyPeer final
203204
if (mySendSize != 0)
204205
return;
205206

206-
// Finally all is sent. Send the next one in 1 second.
207+
// Finally all is sent. Send the next one in 1 second. For that need to actually
208+
// do socket->SetDeadline(), but we will do it in OnEvent().
207209
mySendDeadline = mg::box::GetMilliseconds() + 1000;
208-
// OnEvent() is going to be executed again ASAP, but this is ok. It is simpler
209-
// for us to implement all the logic in one function. It will handle this
210-
// reschedule a spurious wakeup.
210+
// Reschedule() basically means 'reschedule the socket right away'. Same as saying
211+
// SetDeadline(0). OnEvent() is going to be executed again ASAP. This way it is
212+
// simpler for us to implement all the logic in one function.
211213
mySock->Reschedule();
212214
}
213215

0 commit comments

Comments
 (0)