Skip to content

Commit 43069b6

Browse files
lightvectorlightvector
committed
Fix broken silly function referencing with async bot
1 parent cce5ea1 commit 43069b6

File tree

5 files changed

+62
-60
lines changed

5 files changed

+62
-60
lines changed

‎cpp/command/analysis.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,10 +419,10 @@ int MainCmds::analysis(int argc, const char* const* argv) {
419419
const bool isDuringSearch = true;
420420
reportAnalysis(request,search,isDuringSearch);
421421
};
422-
bot->genMoveSynchronousAnalyze(pla, TimeControls(), searchFactor, request->reportDuringSearchEvery, &callback, &onSearchBegun);
422+
bot->genMoveSynchronousAnalyze(pla, TimeControls(), searchFactor, request->reportDuringSearchEvery, callback, onSearchBegun);
423423
}
424424
else {
425-
bot->genMoveSynchronous(pla, TimeControls(), searchFactor, &onSearchBegun);
425+
bot->genMoveSynchronous(pla, TimeControls(), searchFactor, onSearchBegun);
426426
}
427427

428428
if(logSearchInfo) {

‎cpp/command/gtp.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,8 @@ struct GTPEngine {
610610
std::function<void(const Search* search)> callback;
611611
//lz-analyze
612612
if(args.lz && !args.kata) {
613+
//Avoid capturing anything by reference except [this], since this will potentially be used
614+
//asynchronously and called after we return
613615
callback = [args,pla,this](const Search* search) {
614616
vector<AnalysisData> buf;
615617
search->getAnalysisData(buf,args.minMoves,false,analysisPVLen);
@@ -811,7 +813,7 @@ struct GTPEngine {
811813
bot->setAlwaysIncludeOwnerMap(true);
812814
else
813815
bot->setAlwaysIncludeOwnerMap(false);
814-
moveLoc = bot->genMoveSynchronousAnalyze(pla, tc, searchFactor, args.secondsPerReport, &callback);
816+
moveLoc = bot->genMoveSynchronousAnalyze(pla, tc, searchFactor, args.secondsPerReport, callback);
815817
//Make sure callback happens at least once
816818
callback(bot->getSearch());
817819
}
@@ -1054,7 +1056,7 @@ struct GTPEngine {
10541056
bot->setAlwaysIncludeOwnerMap(false);
10551057

10561058
double searchFactor = 1e40; //go basically forever
1057-
bot->analyze(pla, searchFactor, args.secondsPerReport, &callback);
1059+
bot->analyzeAsync(pla, searchFactor, args.secondsPerReport, callback);
10581060
}
10591061

10601062
double computeLead(Logger& logger) {

‎cpp/command/misc.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ int MainCmds::demoplay(int argc, const char* const* argv) {
465465
PlayUtils::getSearchFactor(searchFactorWhenWinningThreshold,searchFactorWhenWinning,params,recentWinLossValues,P_BLACK),
466466
PlayUtils::getSearchFactor(searchFactorWhenWinningThreshold,searchFactorWhenWinning,params,recentWinLossValues,P_WHITE)
467467
);
468-
Loc moveLoc = bot->genMoveSynchronousAnalyze(pla,tc,searchFactor,callbackPeriod,&callback);
468+
Loc moveLoc = bot->genMoveSynchronousAnalyze(pla,tc,searchFactor,callbackPeriod,callback);
469469

470470
bool isLegal = bot->isLegalStrict(moveLoc,pla);
471471
if(moveLoc == Board::NULL_LOC || !isLegal) {

‎cpp/search/asyncbot.cpp‎

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -117,15 +117,15 @@ bool AsyncBot::isLegalStrict(Loc moveLoc, Player movePla) const {
117117
return search->isLegalStrict(moveLoc,movePla);
118118
}
119119

120-
void AsyncBot::genMove(Player movePla, int searchId, const TimeControls& tc, std::function<void(Loc,int)>* onMove) {
121-
genMove(movePla,searchId,tc,1.0,onMove,NULL);
120+
void AsyncBot::genMoveAsync(Player movePla, int searchId, const TimeControls& tc, const std::function<void(Loc,int)>& onMove) {
121+
genMoveAsync(movePla,searchId,tc,1.0,onMove,nullptr);
122122
}
123123

124-
void AsyncBot::genMove(Player movePla, int searchId, const TimeControls& tc, double sf, std::function<void(Loc,int)>* onMove) {
125-
genMove(movePla,searchId,tc,sf,onMove,NULL);
124+
void AsyncBot::genMoveAsync(Player movePla, int searchId, const TimeControls& tc, double sf, const std::function<void(Loc,int)>& onMove) {
125+
genMoveAsync(movePla,searchId,tc,sf,onMove,nullptr);
126126
}
127127

128-
void AsyncBot::genMove(Player movePla, int searchId, const TimeControls& tc, double sf, std::function<void(Loc,int)>* onMove, std::function<void()>* onSearchBegun) {
128+
void AsyncBot::genMoveAsync(Player movePla, int searchId, const TimeControls& tc, double sf, const std::function<void(Loc,int)>& onMove, const std::function<void()>& onSearchBegun) {
129129
unique_lock<std::mutex> lock(controlMutex);
130130
stopAndWaitAlreadyLocked(lock);
131131
assert(!isRunning);
@@ -143,28 +143,28 @@ void AsyncBot::genMove(Player movePla, int searchId, const TimeControls& tc, dou
143143
timeControls = tc;
144144
searchFactor = sf;
145145
analyzeCallbackPeriod = -1;
146-
analyzeCallback = NULL;
146+
analyzeCallback = nullptr;
147147
searchBegunCallback = onSearchBegun;
148148
lock.unlock();
149149
threadWaitingToSearch.notify_all();
150150
}
151151

152152
Loc AsyncBot::genMoveSynchronous(Player movePla, const TimeControls& tc) {
153-
return genMoveSynchronous(movePla,tc,1.0,NULL);
153+
return genMoveSynchronous(movePla,tc,1.0,nullptr);
154154
}
155155

156156
Loc AsyncBot::genMoveSynchronous(Player movePla, const TimeControls& tc, double sf) {
157-
return genMoveSynchronous(movePla,tc,sf,NULL);
157+
return genMoveSynchronous(movePla,tc,sf,nullptr);
158158
}
159159

160-
Loc AsyncBot::genMoveSynchronous(Player movePla, const TimeControls& tc, double sf, std::function<void()>* onSearchBegun) {
160+
Loc AsyncBot::genMoveSynchronous(Player movePla, const TimeControls& tc, double sf, const std::function<void()>& onSearchBegun) {
161161
Loc moveLoc = Board::NULL_LOC;
162162
std::function<void(Loc,int)> onMove = [&moveLoc](Loc loc, int searchId) {
163163
assert(searchId == 0);
164164
(void)searchId; //avoid warning when asserts disabled
165165
moveLoc = loc;
166166
};
167-
genMove(movePla,0,tc,sf,&onMove,onSearchBegun);
167+
genMoveAsync(movePla,0,tc,sf,onMove,onSearchBegun);
168168
waitForSearchToEnd();
169169
return moveLoc;
170170
}
@@ -181,19 +181,19 @@ void AsyncBot::ponder(double sf) {
181181
return;
182182

183183
queuedSearchId = 0;
184-
queuedOnMove = NULL;
184+
queuedOnMove = nullptr;
185185
isRunning = true;
186186
isPondering = true; //True - we are searching on the opponent's turn "for" the opponent's opponent
187187
shouldStopNow = false;
188188
timeControls = TimeControls(); //Blank time controls since opponent's clock is running, not ours, so no cap other than searchFactor
189189
searchFactor = sf;
190190
analyzeCallbackPeriod = -1;
191-
analyzeCallback = NULL;
192-
searchBegunCallback = NULL;
191+
analyzeCallback = nullptr;
192+
searchBegunCallback = nullptr;
193193
lock.unlock();
194194
threadWaitingToSearch.notify_all();
195195
}
196-
void AsyncBot::analyze(Player movePla, double sf, double callbackPeriod, std::function<void(const Search* search)>* callback) {
196+
void AsyncBot::analyzeAsync(Player movePla, double sf, double callbackPeriod, const std::function<void(const Search* search)>& callback) {
197197
unique_lock<std::mutex> lock(controlMutex);
198198
stopAndWaitAlreadyLocked(lock);
199199
assert(!isRunning);
@@ -204,30 +204,30 @@ void AsyncBot::analyze(Player movePla, double sf, double callbackPeriod, std::fu
204204
search->setPlayerAndClearHistory(movePla);
205205

206206
queuedSearchId = 0;
207-
queuedOnMove = NULL;
207+
queuedOnMove = nullptr;
208208
isRunning = true;
209209
isPondering = false; //This should indeed be false because we are searching for the current player, not the last player we did a regular search for.
210210
shouldStopNow = false;
211211
timeControls = TimeControls(); //Blank time controls since no clock is not running, we don't cap search time other than through searchFactor.
212212
searchFactor = sf;
213213
analyzeCallbackPeriod = callbackPeriod;
214214
analyzeCallback = callback;
215-
searchBegunCallback = NULL;
215+
searchBegunCallback = nullptr;
216216
lock.unlock();
217217
threadWaitingToSearch.notify_all();
218218
}
219219

220-
void AsyncBot::genMoveAnalyze(
221-
Player movePla, int searchId, const TimeControls& tc, double sf, std::function<void(Loc,int)>* onMove,
222-
double callbackPeriod, std::function<void(const Search* search)>* callback
220+
void AsyncBot::genMoveAsyncAnalyze(
221+
Player movePla, int searchId, const TimeControls& tc, double sf, const std::function<void(Loc,int)>& onMove,
222+
double callbackPeriod, const std::function<void(const Search* search)>& callback
223223
) {
224-
genMoveAnalyze(movePla, searchId, tc, sf, onMove, callbackPeriod, callback, NULL);
224+
genMoveAsyncAnalyze(movePla, searchId, tc, sf, onMove, callbackPeriod, callback, nullptr);
225225
}
226226

227-
void AsyncBot::genMoveAnalyze(
228-
Player movePla, int searchId, const TimeControls& tc, double sf, std::function<void(Loc,int)>* onMove,
229-
double callbackPeriod, std::function<void(const Search* search)>* callback,
230-
std::function<void()>* onSearchBegun
227+
void AsyncBot::genMoveAsyncAnalyze(
228+
Player movePla, int searchId, const TimeControls& tc, double sf, const std::function<void(Loc,int)>& onMove,
229+
double callbackPeriod, const std::function<void(const Search* search)>& callback,
230+
const std::function<void()>& onSearchBegun
231231
) {
232232
unique_lock<std::mutex> lock(controlMutex);
233233
stopAndWaitAlreadyLocked(lock);
@@ -254,23 +254,23 @@ void AsyncBot::genMoveAnalyze(
254254

255255
Loc AsyncBot::genMoveSynchronousAnalyze(
256256
Player movePla, const TimeControls& tc, double sf,
257-
double callbackPeriod, std::function<void(const Search* search)>* callback
257+
double callbackPeriod, const std::function<void(const Search* search)>& callback
258258
) {
259-
return genMoveSynchronousAnalyze(movePla, tc, sf, callbackPeriod, callback, NULL);
259+
return genMoveSynchronousAnalyze(movePla, tc, sf, callbackPeriod, callback, nullptr);
260260
}
261261

262262
Loc AsyncBot::genMoveSynchronousAnalyze(
263263
Player movePla, const TimeControls& tc, double sf,
264-
double callbackPeriod, std::function<void(const Search* search)>* callback,
265-
std::function<void()>* onSearchBegun
264+
double callbackPeriod, const std::function<void(const Search* search)>& callback,
265+
const std::function<void()>& onSearchBegun
266266
) {
267267
Loc moveLoc = Board::NULL_LOC;
268268
std::function<void(Loc,int)> onMove = [&moveLoc](Loc loc, int searchId) {
269269
assert(searchId == 0);
270270
(void)searchId; //avoid warning when asserts disabled
271271
moveLoc = loc;
272272
};
273-
genMoveAnalyze(movePla,0,tc,sf,&onMove,callbackPeriod,callback,onSearchBegun);
273+
genMoveAsyncAnalyze(movePla,0,tc,sf,onMove,callbackPeriod,callback,onSearchBegun);
274274
waitForSearchToEnd();
275275
return moveLoc;
276276
}
@@ -323,8 +323,8 @@ void AsyncBot::internalSearchThreadLoop() {
323323
TimeControls tc = timeControls;
324324
double callbackPeriod = analyzeCallbackPeriod;
325325
//Make local copies just in case, to simplify thread reasoning for the member fields
326-
std::function<void(const Search*)>* analyzeCallbackLocal = analyzeCallback;
327-
std::function<void()>* searchBegunCallbackLocal = searchBegunCallback;
326+
std::function<void(const Search*)> analyzeCallbackLocal = analyzeCallback;
327+
std::function<void()> searchBegunCallbackLocal = searchBegunCallback;
328328
lock.unlock();
329329

330330
//Make sure we don't feed in absurdly large numbers, this seems to cause wait_for to hang.
@@ -338,8 +338,8 @@ void AsyncBot::internalSearchThreadLoop() {
338338
atomic<bool> isSearchBegun(false);
339339
std::function<void()> searchBegun = [&isSearchBegun,&searchBegunCallbackLocal]() {
340340
isSearchBegun.store(true,std::memory_order_release);
341-
if(searchBegunCallbackLocal != NULL)
342-
(*searchBegunCallbackLocal)();
341+
if(searchBegunCallbackLocal)
342+
searchBegunCallbackLocal();
343343
};
344344
auto callbackLoop = [this,callbackPeriod,&analyzeCallbackLocal,&callbackLoopWaiting,&callbackLoopShouldStop,&isSearchBegun]() {
345345
unique_lock<std::mutex> callbackLock(controlMutex);
@@ -354,14 +354,14 @@ void AsyncBot::internalSearchThreadLoop() {
354354
if(!isSearchBegun.load(std::memory_order_acquire))
355355
continue;
356356
callbackLock.unlock();
357-
(*analyzeCallbackLocal)(search);
357+
analyzeCallbackLocal(search);
358358
callbackLock.lock();
359359
}
360360
callbackLock.unlock();
361361
};
362362

363363
std::thread callbackLoopThread;
364-
if(callbackPeriod >= 0 && analyzeCallbackLocal != NULL) {
364+
if(callbackPeriod >= 0 && analyzeCallbackLocal) {
365365
callbackLoopThread = std::thread(callbackLoop);
366366
}
367367

@@ -378,8 +378,8 @@ void AsyncBot::internalSearchThreadLoop() {
378378

379379
lock.lock();
380380
//Call queuedOnMove under the lock just in case
381-
if(queuedOnMove != NULL)
382-
(*queuedOnMove)(moveLoc,queuedSearchId);
381+
if(queuedOnMove)
382+
queuedOnMove(moveLoc,queuedSearchId);
383383
isRunning = false;
384384
isPondering = false;
385385
userWaitingForStop.notify_all();

‎cpp/search/asyncbot.h‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -50,40 +50,40 @@ class AsyncBot {
5050
//Asynchronously calls the provided function upon success, passing back the move and provided searchId.
5151
//The provided callback is expected to terminate quickly and should NOT call back into this API.
5252
//onSearchBegun is called when the search has initialized its tree, after which many asynchronous search query functions become safe
53-
void genMove(Player movePla, int searchId, const TimeControls& tc, std::function<void(Loc,int)>* onMove);
54-
void genMove(Player movePla, int searchId, const TimeControls& tc, double searchFactor, std::function<void(Loc,int)>* onMove);
55-
void genMove(Player movePla, int searchId, const TimeControls& tc, double searchFactor, std::function<void(Loc,int)>* onMove, std::function<void()>* onSearchBegun);
53+
void genMoveAsync(Player movePla, int searchId, const TimeControls& tc, const std::function<void(Loc,int)>& onMove);
54+
void genMoveAsync(Player movePla, int searchId, const TimeControls& tc, double searchFactor, const std::function<void(Loc,int)>& onMove);
55+
void genMoveAsync(Player movePla, int searchId, const TimeControls& tc, double searchFactor, const std::function<void(Loc,int)>& onMove, const std::function<void()>& onSearchBegun);
5656

5757
//Same as genMove, but waits directly for the move and returns it here.
5858
Loc genMoveSynchronous(Player movePla, const TimeControls& tc);
5959
Loc genMoveSynchronous(Player movePla, const TimeControls& tc, double searchFactor);
60-
Loc genMoveSynchronous(Player movePla, const TimeControls& tc, double searchFactor, std::function<void()>* onSearchBegun);
60+
Loc genMoveSynchronous(Player movePla, const TimeControls& tc, double searchFactor, const std::function<void()>& onSearchBegun);
6161

6262
//Begin pondering, returning immediately. Future genMoves may be faster if this is called.
6363
//Will not stop any ongoing searches.
6464
void ponder();
6565
void ponder(double searchFactor);
6666

6767
//Terminate any existing searches, and then begin pondering while periodically calling the specified callback
68-
void analyze(Player movePla, double searchFactor, double callbackPeriod, std::function<void(const Search* search)>* callback);
68+
void analyzeAsync(Player movePla, double searchFactor, double callbackPeriod, const std::function<void(const Search* search)>& callback);
6969
//Same as genMove but with periodic analyze callbacks
70-
void genMoveAnalyze(
71-
Player movePla, int searchId, const TimeControls& tc, double searchFactor, std::function<void(Loc,int)>* onMove,
72-
double callbackPeriod, std::function<void(const Search* search)>* callback
70+
void genMoveAsyncAnalyze(
71+
Player movePla, int searchId, const TimeControls& tc, double searchFactor, const std::function<void(Loc,int)>& onMove,
72+
double callbackPeriod, const std::function<void(const Search* search)>& callback
7373
);
74-
void genMoveAnalyze(
75-
Player movePla, int searchId, const TimeControls& tc, double searchFactor, std::function<void(Loc,int)>* onMove,
76-
double callbackPeriod, std::function<void(const Search* search)>* callback,
77-
std::function<void()>* onSearchBegun
74+
void genMoveAsyncAnalyze(
75+
Player movePla, int searchId, const TimeControls& tc, double searchFactor, const std::function<void(Loc,int)>& onMove,
76+
double callbackPeriod, const std::function<void(const Search* search)>& callback,
77+
const std::function<void()>& onSearchBegun
7878
);
7979
Loc genMoveSynchronousAnalyze(
8080
Player movePla, const TimeControls& tc, double searchFactor,
81-
double callbackPeriod, std::function<void(const Search* search)>* callback
81+
double callbackPeriod, const std::function<void(const Search* search)>& callback
8282
);
8383
Loc genMoveSynchronousAnalyze(
8484
Player movePla, const TimeControls& tc, double searchFactor,
85-
double callbackPeriod, std::function<void(const Search* search)>* callback,
86-
std::function<void()>* onSearchBegun
85+
double callbackPeriod, const std::function<void(const Search* search)>& callback,
86+
const std::function<void()>& onSearchBegun
8787
);
8888

8989
//Signal an ongoing genMove or ponder to stop as soon as possible, and wait for the stop to happen.
@@ -110,12 +110,12 @@ class AsyncBot {
110110
bool isKilled;
111111
std::atomic<bool> shouldStopNow;
112112
int queuedSearchId;
113-
std::function<void(Loc,int)>* queuedOnMove;
113+
std::function<void(Loc,int)> queuedOnMove;
114114
TimeControls timeControls;
115115
double searchFactor;
116116
double analyzeCallbackPeriod;
117-
std::function<void(const Search* search)>* analyzeCallback;
118-
std::function<void()>* searchBegunCallback;
117+
std::function<void(const Search* search)> analyzeCallback;
118+
std::function<void()> searchBegunCallback;
119119

120120
void stopAndWaitAlreadyLocked(std::unique_lock<std::mutex>& lock);
121121
void waitForSearchToEnd();

0 commit comments

Comments
 (0)