Skip to content

Commit 0198c9a

Browse files
author
Yorhel
committed
dlfile: Detect and handle I/O errors during finalization
Correctly handling, and more importantly, recovering from any possible I/O errors is a huge pain and fairly complex. The current fixes should allow full recovery when the move to the final destination fails, even across ncdc restarts. But there's probably still some error sequences that can't be recovered from. That's hard/impossible to really fix, because the ftruncate() and file move can't be done as a single atomic operation. Also, recovery across restarts assumes that the changes to the db.sqlite3 file do get flushed.
1 parent b872d03 commit 0198c9a

File tree

1 file changed

+39
-13
lines changed

1 file changed

+39
-13
lines changed

src/dlfile.c

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,11 @@ static gboolean dlfile_save_bitmap_timeout(gpointer dat) {
121121
dl_t *dl = dat;
122122
g_static_mutex_lock(&dl->lock);
123123
dl->bitmap_src = 0;
124-
if(dl->incfd >= 0 && !dlfile_save_bitmap(dl, dl->incfd)) {
124+
if(dl->incfd > 0 && !dlfile_save_bitmap(dl, dl->incfd)) {
125125
g_warning("Error writing bitmap for `%s': %s.", dl->dest, g_strerror(errno));
126126
dl_queue_seterr(dl, DLE_IO_INC, g_strerror(errno));
127127
}
128-
if(dl->incfd >= 0 && !dl->active_threads) {
128+
if(dl->incfd > 0 && !dl->active_threads) {
129129
close(dl->incfd);
130130
dl->incfd = 0;
131131
}
@@ -290,6 +290,18 @@ static gboolean dlfile_load_threads(dl_t *dl, int fd) {
290290

291291

292292
void dlfile_load(dl_t *dl) {
293+
/* If moving to the destination failed in a previous run, assume that the
294+
* incoming file is complete and all that's left to do is resume the
295+
* finalization (which the user has to initiate by clearing the error). This
296+
* needs to be handled as a special case because the incoming file will not
297+
* contain the bitmap anymore at this point, and the loading process below
298+
* will fail. */
299+
if(dl->prio == DLP_ERR && dl->error == DLE_IO_DEST) {
300+
g_message("Download for `%s' in IO_DEST error state, assuming `%s' contains the finished download.", dl->dest, dl->inc);
301+
dl->have = dl->size;
302+
return;
303+
}
304+
293305
dl->have = 0;
294306
int fd = open(dl->inc, O_RDWR);
295307
if(fd < 0) {
@@ -352,8 +364,8 @@ static gboolean dlfile_open(dl_t *dl) {
352364
return FALSE;
353365
}
354366

355-
/* Everything else has already been initialized if we have a thread */
356-
if(dl->threads)
367+
/* Everything else has already been initialized if we have a thread or bitmap */
368+
if(dl->threads || dl->bitmap)
357369
return TRUE;
358370

359371
if(!dl->islist) {
@@ -387,20 +399,33 @@ void dlfile_finished(dl_t *dl) {
387399
/* Regular files: Remove bitmap from the file
388400
* File lists: Ensure that the file size is correct after we've downloaded a
389401
* longer file list before that got interrupted. */
390-
/* TODO: Error handling */
391-
ftruncate(dl->incfd, dl->size);
392-
close(dl->incfd);
402+
if(ftruncate(dl->incfd, dl->size) < 0) {
403+
g_warning("Error truncating the incoming file for `%s': %s.", dl->dest, g_strerror(errno));
404+
dl_queue_seterr(dl, DLE_IO_INC, g_strerror(errno));
405+
return;
406+
}
407+
int r = close(dl->incfd);
393408
dl->incfd = 0;
409+
if(r < 0) {
410+
g_warning("Error closing the incoming file for `%s': %s.", dl->dest, g_strerror(errno));
411+
dl_queue_seterr(dl, DLE_IO_INC, g_strerror(errno));
412+
return;
413+
}
394414

395415
char *fdest = g_filename_from_utf8(dl->dest, -1, NULL, NULL, NULL);
396416
if(!fdest)
397417
fdest = g_strdup(dl->dest);
398418

399419
/* Create destination directory, if it does not exist yet. */
400420
char *parent = g_path_get_dirname(fdest);
401-
g_mkdir_with_parents(parent, 0755);
402-
/* TODO: Error handling */
421+
r = g_mkdir_with_parents(parent, 0755);
403422
g_free(parent);
423+
if(r < 0) {
424+
g_warning("Error creating directory for `%s': %s.", dl->dest, g_strerror(errno));
425+
dl_queue_seterr(dl, DLE_IO_DEST, g_strerror(errno));
426+
g_free(fdest);
427+
return;
428+
}
404429

405430
/* Prevent overwiting other files by appending a prefix to the destination if
406431
* it already exists. It is assumed that fn + any dupe-prevention-extension
@@ -412,17 +437,18 @@ void dlfile_finished(dl_t *dl) {
412437
g_free(dest);
413438
dest = g_strdup_printf("%s.%d", fdest, num++);
414439
}
440+
g_free(fdest);
415441

416442
GError *err = NULL;
417443
file_move(dl->inc, dest, dl->islist, &err);
444+
g_free(dest);
418445
if(err) {
419-
/* TODO: Error handling. */
446+
g_warning("Error moving file to destination `%s': %s.", dl->dest, err->message);
447+
dl_queue_seterr(dl, DLE_IO_DEST, err->message);
420448
g_error_free(err);
449+
return;
421450
}
422451

423-
g_free(dest);
424-
g_free(fdest);
425-
426452
dl_finished(dl);
427453
}
428454

0 commit comments

Comments
 (0)