Skip to content

Commit f807f31

Browse files
committed
fix race condition in bk_download verify;
Signed-off-by: yuchen.cc <[email protected]>
1 parent 5abd815 commit f807f31

File tree

3 files changed

+21
-16
lines changed

3 files changed

+21
-16
lines changed

src/bk_download.cpp

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,17 @@ bool BkDownload::download_done() {
109109
new_name = dir + "/" + COMMIT_FILE_NAME;
110110

111111
// verify sha256
112-
auto th = photon::CURRENT;
112+
photon::semaphore done;
113113
std::string shares;
114-
std::thread sha256_thread([&, th]() {
114+
std::thread sha256_thread([&]() {
115115
shares = sha256sum(old_name.c_str());
116-
photon::thread_interrupt(th, EINTR);
116+
done.signal(1);
117117
});
118118
sha256_thread.detach();
119-
photon::thread_usleep(-1UL);
119+
while (running == 1) {
120+
if (done.wait(1, 200 * 1000) == 0)
121+
break;
122+
}
120123
if (shares != digest) {
121124
LOG_ERROR("verify checksum ` failed (expect: `, got: `)", old_name, digest, shares);
122125
force_download = true; // force redownload next time
@@ -128,17 +131,17 @@ bool BkDownload::download_done() {
128131
LOG_ERROR("rename(`,`), `:`", old_name, new_name, errno, strerror(errno));
129132
return false;
130133
}
131-
LOG_INFO("download done. rename(`,`) success", old_name, new_name);
134+
LOG_INFO("download verify done. rename(`,`) success", old_name, new_name);
132135
return true;
133136
}
134137

135-
bool BkDownload::download(int &running) {
138+
bool BkDownload::download() {
136139
if (check_downloaded(dir)) {
137140
switch_to_local_file();
138141
return true;
139142
}
140143

141-
if (download_blob(running)) {
144+
if (download_blob()) {
142145
if (!download_done())
143146
return false;
144147
switch_to_local_file();
@@ -149,7 +152,7 @@ bool BkDownload::download(int &running) {
149152

150153
bool BkDownload::lock_file() {
151154
if (lock_files.find(dir) != lock_files.end()) {
152-
LOG_WARN("failded to lock download path:`", dir);
155+
LOG_WARN("failed to lock download path:`", dir);
153156
return false;
154157
}
155158
lock_files.insert(dir);
@@ -160,7 +163,7 @@ void BkDownload::unlock_file() {
160163
lock_files.erase(dir);
161164
}
162165

163-
bool BkDownload::download_blob(int &running) {
166+
bool BkDownload::download_blob() {
164167
std::string dl_file_path = dir + "/" + DOWNLOAD_TMP_NAME;
165168
try_cnt--;
166169
IFile *src = src_file;
@@ -231,6 +234,7 @@ bool BkDownload::download_blob(int &running) {
231234
}
232235
offset += count;
233236
}
237+
LOG_INFO("download blob done. (`)", dl_file_path);
234238
return true;
235239
}
236240

@@ -260,7 +264,7 @@ void bk_download_proc(std::list<BKDL::BkDownload *> &dl_list, uint64_t delay_sec
260264
continue;
261265
}
262266

263-
bool succ = dl_item->download(running);
267+
bool succ = dl_item->download();
264268
dl_item->unlock_file();
265269

266270
if (running != 1) {
@@ -287,7 +291,7 @@ void bk_download_proc(std::list<BKDL::BkDownload *> &dl_list, uint64_t delay_sec
287291
delete dl_item;
288292
}
289293
}
290-
LOG_DEBUG("BACKGROUND DOWNLOAD THREAD EXIT.");
294+
LOG_INFO("BACKGROUND DOWNLOAD THREAD EXIT.");
291295
}
292296

293297
} // namespace BKDL

src/bk_download.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class BkDownload {
3232
std::string dir;
3333
uint32_t try_cnt;
3434

35-
bool download(int &running);
35+
bool download();
3636
bool lock_file();
3737
void unlock_file();
3838

@@ -43,14 +43,14 @@ class BkDownload {
4343
}
4444
BkDownload(ISwitchFile *sw_file, photon::fs::IFile *src_file, size_t file_size,
4545
const std::string dir, int32_t limit_MB_ps, int32_t try_cnt, ImageFile *image_file,
46-
std::string digest)
46+
std::string digest, int &running)
4747
: sw_file(sw_file), src_file(src_file), file_size(file_size), dir(dir),
48-
limit_MB_ps(limit_MB_ps), try_cnt(try_cnt), image_file(image_file), digest(digest) {
48+
limit_MB_ps(limit_MB_ps), try_cnt(try_cnt), image_file(image_file), digest(digest), running(running) {
4949
}
5050

5151
private:
5252
void switch_to_local_file();
53-
bool download_blob(int &running);
53+
bool download_blob();
5454
bool download_done();
5555

5656
ISwitchFile *sw_file = nullptr;
@@ -60,6 +60,7 @@ class BkDownload {
6060
std::string digest;
6161
size_t file_size;
6262
bool force_download = false;
63+
int &running;
6364
};
6465

6566
void bk_download_proc(std::list<BKDL::BkDownload *> &, uint64_t, int &);

src/image_file.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ IFile *ImageFile::__open_ro_remote(const std::string &dir, const std::string &di
185185
} else {
186186
BKDL::BkDownload *obj =
187187
new BKDL::BkDownload(switch_file, srcfile, size, dir, conf.download().maxMBps(),
188-
conf.download().tryCnt(), this, digest);
188+
conf.download().tryCnt(), this, digest, m_status);
189189
LOG_DEBUG("add to download list for `", dir);
190190
dl_list.push_back(obj);
191191
}

0 commit comments

Comments
 (0)