Skip to content

Commit 7286314

Browse files
committed
cache, link: support concurrent downloads in cache mode
Replace the single active download state tracking in Cache with a linked list of ActiveDownload structs. This enables multiple background threads to download different segments concurrently in cache mode, resolving race conditions and clobbered trackers. Key changes: - Add ActiveDownload struct (offset, ts, next) and active_dls list to Cache, replacing the scalar active_dl_offset and active_dl_ts fields - Add ActiveDownload_find, ActiveDownload_add, and ActiveDownload_remove helpers to manage the list under dl_lock - Implement double-checked locking in Cache_read_segment to prevent duplicate background downloads under concurrent FUSE workers - Guard next-segment prefetch Seg_exist checks with w_lock to avoid data races with concurrent segment updates - Refactor Cache_bgdl_launcher to accept a BgdlArg struct allocated on the heap, eliminating the write-only next_dl_offset field and preventing data races when passing the target offset - Update Link_download in link.c to use ActiveDownload_find when registering and clearing transfer structs
1 parent 0228b55 commit 7286314

3 files changed

Lines changed: 204 additions & 68 deletions

File tree

src/cache.c

Lines changed: 174 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,52 @@ int CacheDir_create(const char *dirn)
541541
FREE(metadirn);
542542
return res;
543543
}
544+
ActiveDownload *ActiveDownload_find(Cache *cf, off_t offset)
545+
{
546+
ActiveDownload *ad = cf->active_dls;
547+
while (ad) {
548+
if (ad->offset == offset) {
549+
return ad;
550+
}
551+
ad = ad->next;
552+
}
553+
return NULL;
554+
}
555+
556+
/**
557+
* \brief Allocates and prepends a new ActiveDownload tracker to the list.
558+
* \param[in] cf The cache instance.
559+
* \param[in] offset The offset to track.
560+
* \note Must be called while holding cf->dl_lock.
561+
*/
562+
static void ActiveDownload_add(Cache *cf, off_t offset)
563+
{
564+
ActiveDownload *ad = CALLOC(1, sizeof(ActiveDownload));
565+
ad->offset = offset;
566+
ad->ts = NULL;
567+
ad->next = cf->active_dls;
568+
cf->active_dls = ad;
569+
}
570+
571+
/**
572+
* \brief Removes and frees the ActiveDownload tracker for the given offset.
573+
* \param[in] cf The cache instance.
574+
* \param[in] offset The offset to untrack.
575+
* \note Must be called while holding cf->dl_lock.
576+
*/
577+
static void ActiveDownload_remove(Cache *cf, off_t offset)
578+
{
579+
ActiveDownload **curr = &cf->active_dls;
580+
while (*curr) {
581+
if ((*curr)->offset == offset) {
582+
ActiveDownload *temp = *curr;
583+
*curr = (*curr)->next;
584+
FREE(temp);
585+
return;
586+
}
587+
curr = &(*curr)->next;
588+
}
589+
}
544590

545591
/**
546592
* \brief Allocate a new cache data structure
@@ -552,8 +598,7 @@ static Cache *Cache_alloc(void)
552598
PTHREAD_MUTEX_INIT(&cf->w_lock, NULL);
553599
PTHREAD_MUTEX_INIT(&cf->dl_lock, NULL);
554600
PTHREAD_COND_INIT(&cf->dl_cond, NULL);
555-
cf->active_dl_offset = -1;
556-
cf->active_dl_ts = NULL;
601+
cf->active_dls = NULL;
557602
cf->cache_opened = 1;
558603
SEM_INIT(&cf->bgt_sem, 0, 1);
559604
return cf;
@@ -582,6 +627,13 @@ static void Cache_free(Cache *cf)
582627
FREE(cf->fs_path);
583628
}
584629

630+
ActiveDownload *ad = cf->active_dls;
631+
while (ad) {
632+
ActiveDownload *next = ad->next;
633+
FREE(ad);
634+
ad = next;
635+
}
636+
585637
FREE(cf);
586638
}
587639

@@ -984,19 +1036,27 @@ static void Seg_set(Cache *cf, off_t offset, int i)
9841036
cf->seg[byte] = i;
9851037
}
9861038

1039+
/**
1040+
* \brief Arguments passed to the background download thread.
1041+
*/
1042+
typedef struct BgdlArg {
1043+
Cache *cf; /**< The cache instance. */
1044+
off_t dl_offset; /**< The segment offset to download. */
1045+
} BgdlArg;
1046+
9871047
/**
9881048
* \brief Background download function
9891049
* \details If we are requesting the data from the second half of the current
9901050
* segment, we can spawn a pthread using this function to download the next
9911051
* segment.
1052+
* \param[in] arg A pointer to a BgdlArg structure.
9921053
*/
9931054
static void *Cache_bgdl(void *arg)
9941055
{
995-
Cache *cf = (Cache *)arg;
996-
997-
PTHREAD_MUTEX_LOCK(&cf->dl_lock);
998-
off_t dl_offset = cf->next_dl_offset;
999-
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
1056+
BgdlArg *bg_arg = (BgdlArg *)arg;
1057+
Cache *cf = bg_arg->cf;
1058+
off_t dl_offset = bg_arg->dl_offset;
1059+
FREE(bg_arg);
10001060

10011061
uint8_t *recv_buf = CALLOC(cf->blksz, sizeof(uint8_t));
10021062
long recv
@@ -1008,10 +1068,8 @@ static void *Cache_bgdl(void *arg)
10081068
(unsigned long)pthread_self(), recv);
10091069
FREE(recv_buf);
10101070
PTHREAD_MUTEX_LOCK(&cf->dl_lock);
1011-
if (cf->active_dl_offset == dl_offset) {
1012-
cf->active_dl_offset = -1;
1013-
PTHREAD_COND_BROADCAST(&cf->dl_cond);
1014-
}
1071+
ActiveDownload_remove(cf, dl_offset);
1072+
PTHREAD_COND_BROADCAST(&cf->dl_cond);
10151073
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
10161074
SEM_POST(&cf->bgt_sem);
10171075
pthread_exit(NULL);
@@ -1034,18 +1092,21 @@ static void *Cache_bgdl(void *arg)
10341092
FREE(recv_buf);
10351093

10361094
PTHREAD_MUTEX_LOCK(&cf->dl_lock);
1037-
if (cf->active_dl_offset == dl_offset) {
1038-
cf->active_dl_offset = -1;
1039-
PTHREAD_COND_BROADCAST(&cf->dl_cond);
1040-
}
1095+
ActiveDownload_remove(cf, dl_offset);
1096+
PTHREAD_COND_BROADCAST(&cf->dl_cond);
10411097
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
10421098

10431099
SEM_POST(&cf->bgt_sem);
10441100

10451101
pthread_exit(NULL);
10461102
}
10471103

1048-
static void Cache_bgdl_launcher(Cache *cf)
1104+
/**
1105+
* \brief Spawns a background thread to download the next segment.
1106+
* \param[in] cf The cache instance.
1107+
* \param[in] dl_offset The offset of the segment to download.
1108+
*/
1109+
static void Cache_bgdl_launcher(Cache *cf, off_t dl_offset)
10491110
{
10501111
pthread_t thread;
10511112
pthread_attr_t attr;
@@ -1059,7 +1120,12 @@ static void Cache_bgdl_launcher(Cache *cf)
10591120
strerror(errno));
10601121
}
10611122

1062-
if (pthread_create(&thread, &attr, Cache_bgdl, cf)) {
1123+
BgdlArg *arg = CALLOC(1, sizeof(BgdlArg));
1124+
arg->cf = cf;
1125+
arg->dl_offset = dl_offset;
1126+
1127+
if (pthread_create(&thread, &attr, Cache_bgdl, arg)) {
1128+
FREE(arg);
10631129
lprintf(fatal, "pthread_create(): %d, %s\n", errno, strerror(errno));
10641130
}
10651131

@@ -1069,6 +1135,46 @@ static void Cache_bgdl_launcher(Cache *cf)
10691135
}
10701136
}
10711137

1138+
/**
1139+
* \brief Reads a segment of size 'len' starting at 'offset_start'.
1140+
* \param[in] cf The cache instance.
1141+
* \param[out] output_buf Output buffer to read data into.
1142+
* \param[in] len Length of the data to read.
1143+
* \param[in] offset_start The offset to start reading from.
1144+
* \return The number of bytes read, or negative on error.
1145+
*
1146+
* \details Concurrency Architecture & State Machine:
1147+
* ----------------------------------------
1148+
* This function supports multiple concurrent segment downloads to allow
1149+
* parallel FUSE reading. To prevent race conditions, memory leaks, and
1150+
* duplicate downloads:
1151+
*
1152+
* 1. Mutual Exclusion & Locking:
1153+
* - `w_lock` guards writing to the cache files (`dfp`/`mfp`) and checking
1154+
* segment existence.
1155+
* - `dl_lock` guards access to `active_dls`, which tracks currently active
1156+
* downloads.
1157+
* - Ordering: ALWAYS lock `w_lock` before `dl_lock` to avoid deadlocks.
1158+
*
1159+
* 2. Active Download Tracking (`active_dls`):
1160+
* - A linked list of `ActiveDownload` nodes tracks the offsets currently
1161+
* being downloaded.
1162+
* - If a segment is not cached but is already being downloaded by another
1163+
* thread, subsequent FUSE threads will detect the node and wait via
1164+
* `PTHREAD_COND_WAIT` on `dl_cond`.
1165+
*
1166+
* 3. Early-Return Copy:
1167+
* - While waiting, threads can perform early returns by copying data
1168+
* directly from the in-progress `TransferStruct`'s memory buffer (`ts->data`)
1169+
* once enough bytes have been received.
1170+
*
1171+
* 4. Double-Checked Locking:
1172+
* - Because locks must be released when launching threads or checking
1173+
* semaphores, other threads could concurrently insert download trackers. We use
1174+
* double-checked locking inside the background thread launcher and `sync_dl`
1175+
* fallback path to verify that the download is still not tracked before
1176+
* allocating a new node.
1177+
*/
10721178
static long Cache_read_segment(Cache *cf, char *const output_buf,
10731179
const off_t len, const off_t offset_start)
10741180
{
@@ -1085,15 +1191,15 @@ static long Cache_read_segment(Cache *cf, char *const output_buf,
10851191
}
10861192

10871193
PTHREAD_MUTEX_LOCK(&cf->dl_lock);
1088-
if (cf->active_dl_offset == dl_offset) {
1194+
ActiveDownload *ad = ActiveDownload_find(cf, dl_offset);
1195+
if (ad != NULL) {
10891196
PTHREAD_MUTEX_UNLOCK(&cf->w_lock);
10901197

1091-
while (cf->active_dl_offset == dl_offset) {
1092-
if (cf->active_dl_ts
1093-
&& cf->active_dl_ts->curr_size
1198+
while ((ad = ActiveDownload_find(cf, dl_offset)) != NULL) {
1199+
if (ad->ts
1200+
&& ad->ts->curr_size
10941201
>= (size_t)(offset_start - dl_offset + len)) {
1095-
memcpy(output_buf,
1096-
cf->active_dl_ts->data + (offset_start - dl_offset),
1202+
memcpy(output_buf, ad->ts->data + (offset_start - dl_offset),
10971203
len);
10981204
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
10991205
send = len;
@@ -1112,29 +1218,46 @@ static long Cache_read_segment(Cache *cf, char *const output_buf,
11121218
}
11131219
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
11141220

1221+
/*
1222+
* Attempt to launch a background download thread if the background thread
1223+
* slot is available (bgt_sem > 0). This acts as a throttle on concurrent
1224+
* background prefetches.
1225+
*/
11151226
ret = sem_trywait(&cf->bgt_sem);
11161227
if (ret == 0) {
11171228
PTHREAD_MUTEX_LOCK(&cf->dl_lock);
1118-
if (cf->active_dl_offset == -1) {
1119-
cf->active_dl_offset = dl_offset;
1120-
cf->next_dl_offset = dl_offset;
1121-
cf->active_dl_ts = NULL;
1229+
/*
1230+
* Double-checked locking: Re-verify that another thread hasn't already
1231+
* added this offset to the active downloads list while we were
1232+
* unlocked.
1233+
*/
1234+
ActiveDownload *bg_ad = ActiveDownload_find(cf, dl_offset);
1235+
if (bg_ad == NULL) {
1236+
ActiveDownload_add(cf, dl_offset);
11221237
PTHREAD_COND_BROADCAST(&cf->dl_cond);
11231238
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
1124-
Cache_bgdl_launcher(cf);
1239+
Cache_bgdl_launcher(cf, dl_offset);
11251240
PTHREAD_MUTEX_UNLOCK(&cf->w_lock);
11261241
goto retry;
11271242
}
11281243
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
1129-
SEM_POST(&cf->bgt_sem);
1244+
SEM_POST(&cf->bgt_sem); /* Back off and release the slot */
11301245
}
11311246

11321247
sync_dl:
11331248
PTHREAD_MUTEX_LOCK(&cf->dl_lock);
1134-
if (cf->active_dl_offset == -1) {
1135-
cf->active_dl_offset = dl_offset;
1249+
/*
1250+
* Double-checked locking: Verify that another thread hasn't concurrently
1251+
* started a download for this offset. If it has, back off, release the
1252+
* locks, and retry to join the wait loop.
1253+
*/
1254+
ActiveDownload *sync_ad = ActiveDownload_find(cf, dl_offset);
1255+
if (sync_ad != NULL) {
1256+
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
1257+
PTHREAD_MUTEX_UNLOCK(&cf->w_lock);
1258+
goto retry;
11361259
}
1137-
cf->active_dl_ts = NULL;
1260+
ActiveDownload_add(cf, dl_offset);
11381261
PTHREAD_COND_BROADCAST(&cf->dl_cond);
11391262
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
11401263

@@ -1148,10 +1271,8 @@ static long Cache_read_segment(Cache *cf, char *const output_buf,
11481271

11491272
if (recv < 0) {
11501273
PTHREAD_MUTEX_LOCK(&cf->dl_lock);
1151-
if (cf->active_dl_offset == dl_offset) {
1152-
cf->active_dl_offset = -1;
1153-
PTHREAD_COND_BROADCAST(&cf->dl_cond);
1154-
}
1274+
ActiveDownload_remove(cf, dl_offset);
1275+
PTHREAD_COND_BROADCAST(&cf->dl_cond);
11551276
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
11561277
FREE(recv_buf);
11571278
PTHREAD_MUTEX_UNLOCK(&cf->w_lock);
@@ -1164,10 +1285,8 @@ static long Cache_read_segment(Cache *cf, char *const output_buf,
11641285
"received %ld bytes, but required at least %ld bytes\n",
11651286
recv, (long)((offset_start - dl_offset) + len));
11661287
PTHREAD_MUTEX_LOCK(&cf->dl_lock);
1167-
if (cf->active_dl_offset == dl_offset) {
1168-
cf->active_dl_offset = -1;
1169-
PTHREAD_COND_BROADCAST(&cf->dl_cond);
1170-
}
1288+
ActiveDownload_remove(cf, dl_offset);
1289+
PTHREAD_COND_BROADCAST(&cf->dl_cond);
11711290
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
11721291
FREE(recv_buf);
11731292
PTHREAD_MUTEX_UNLOCK(&cf->w_lock);
@@ -1182,21 +1301,17 @@ static long Cache_read_segment(Cache *cf, char *const output_buf,
11821301
"error.\n",
11831302
recv, cf->blksz);
11841303
PTHREAD_MUTEX_LOCK(&cf->dl_lock);
1185-
if (cf->active_dl_offset == dl_offset) {
1186-
cf->active_dl_offset = -1;
1187-
PTHREAD_COND_BROADCAST(&cf->dl_cond);
1188-
}
1304+
ActiveDownload_remove(cf, dl_offset);
1305+
PTHREAD_COND_BROADCAST(&cf->dl_cond);
11891306
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
11901307
FREE(recv_buf);
11911308
PTHREAD_MUTEX_UNLOCK(&cf->w_lock);
11921309
return -EIO;
11931310
}
11941311

11951312
PTHREAD_MUTEX_LOCK(&cf->dl_lock);
1196-
if (cf->active_dl_offset == dl_offset) {
1197-
cf->active_dl_offset = -1;
1198-
PTHREAD_COND_BROADCAST(&cf->dl_cond);
1199-
}
1313+
ActiveDownload_remove(cf, dl_offset);
1314+
PTHREAD_COND_BROADCAST(&cf->dl_cond);
12001315
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
12011316
send = len;
12021317
if (offset_start < dl_offset
@@ -1213,17 +1328,22 @@ static long Cache_read_segment(Cache *cf, char *const output_buf,
12131328
bgdl: {
12141329
}
12151330
off_t next_dl_offset = dl_offset + cf->blksz;
1216-
if (!Seg_exist(cf, next_dl_offset) && next_dl_offset < cf->content_length) {
1331+
int next_seg_missing = 0;
1332+
if (next_dl_offset < cf->content_length) {
1333+
PTHREAD_MUTEX_LOCK(&cf->w_lock);
1334+
next_seg_missing = !Seg_exist(cf, next_dl_offset);
1335+
PTHREAD_MUTEX_UNLOCK(&cf->w_lock);
1336+
}
1337+
if (next_seg_missing) {
12171338
ret = sem_trywait(&cf->bgt_sem);
12181339
if (!ret) {
12191340
PTHREAD_MUTEX_LOCK(&cf->dl_lock);
1220-
if (cf->active_dl_offset == -1) {
1221-
cf->active_dl_offset = next_dl_offset;
1222-
cf->next_dl_offset = next_dl_offset;
1223-
cf->active_dl_ts = NULL;
1341+
ActiveDownload *next_ad = ActiveDownload_find(cf, next_dl_offset);
1342+
if (next_ad == NULL) {
1343+
ActiveDownload_add(cf, next_dl_offset);
12241344
PTHREAD_COND_BROADCAST(&cf->dl_cond);
12251345
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
1226-
Cache_bgdl_launcher(cf);
1346+
Cache_bgdl_launcher(cf, next_dl_offset);
12271347
} else {
12281348
PTHREAD_MUTEX_UNLOCK(&cf->dl_lock);
12291349
SEM_POST(&cf->bgt_sem);

0 commit comments

Comments
 (0)