Skip to content

Commit f09fd4b

Browse files
committed
Fix hard download error on 404 etc
I've observed errors when some mirrors are not completely synced. The library tries to download a file, but gets a 404 error. This means we need to retry with another mirror, not crash out. This was crashing because setting `err` in `clean_transcode()` was firing the assert at the start of `truncate_transfer_file()`. Note this failure mode was most common with 404's, but any transfer error could likely have turned fatal, for invalid reasons. We use `cleanup_transcode()` in two contexts. 1. Within `check_transfer_statuses()`. The first call here happens during a normal download after `check_finished_transfer_status()`. The cleanup checks for errors, and any here will be flagged as a `transfer_err` (not a general, err). 2. In 3 other places where an error has already occurred. We need to wait for the program to exit (and it should stop due to a SIGPIPE or short read from stdin), but we don't need to set an error because something already is handling one. It doesn't matter that the transcoder crashed because we're not going to use the output anyway, and we are likely to retry.
1 parent 9351a9a commit f09fd4b

File tree

1 file changed

+14
-10
lines changed

1 file changed

+14
-10
lines changed

librepo/downloader.c

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1545,8 +1545,12 @@ maybe_transcode(LrTarget *target, GError **err)
15451545
}
15461546

15471547
void
1548-
cleanup_transcode(LrTarget *target, GError **err)
1548+
cleanup_transcode(LrTarget *target, GError **transfer_err)
15491549
{
1550+
/** transfer_err can be NULL if we're using this to clean up a failed
1551+
* transfer. In that circumstance g_set_error does nothing which is fine,
1552+
* we don't need to pile on a second failure reason.
1553+
*/
15501554
int wstatus, trc;
15511555
if (!target->writef) {
15521556
return;
@@ -1556,21 +1560,21 @@ cleanup_transcode(LrTarget *target, GError **err)
15561560
}
15571561
fclose(target->writef);
15581562
if(waitpid(target->pid, &wstatus, 0) == -1) {
1559-
g_set_error(err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
1563+
g_set_error(transfer_err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
15601564
"transcode waitpid failed: %s", g_strerror(errno));
15611565
} else if (WIFEXITED(wstatus)) {
15621566
trc = WEXITSTATUS(wstatus);
15631567
if (trc != 0) {
1564-
g_set_error(err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
1568+
g_set_error(transfer_err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
15651569
"transcode process non-zero exit code %d", trc);
15661570
}
15671571
} else if (WIFSIGNALED(wstatus)) {
1568-
g_set_error(err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
1572+
g_set_error(transfer_err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
15691573
"transcode process was terminated with a signal: %d",
15701574
WTERMSIG(wstatus));
15711575
} else {
15721576
/* don't think this can happen, but covering all bases */
1573-
g_set_error(err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
1577+
g_set_error(transfer_err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
15741578
"transcode unhandled circumstance in waitpid");
15751579
}
15761580
target->writef = NULL;
@@ -1837,7 +1841,7 @@ prepare_next_transfer(LrDownload *dd, gboolean *candidatefound, GError **err)
18371841
curl_easy_cleanup(target->curl_handle);
18381842
target->curl_handle = NULL;
18391843
}
1840-
cleanup_transcode(target, err);
1844+
cleanup_transcode(target, NULL);
18411845
if (target->f != NULL) {
18421846
fclose(target->f);
18431847
target->f = NULL;
@@ -2405,11 +2409,11 @@ check_transfer_statuses(LrDownload *dd, GError **err)
24052409
if (!ret) // Error
24062410
return FALSE;
24072411

2412+
cleanup_transcode(target, &transfer_err);
2413+
24082414
if (transfer_err) // Transfer was unsuccessful
24092415
goto transfer_error;
24102416

2411-
cleanup_transcode(target, err);
2412-
24132417
//
24142418
// Checksum checking
24152419
//
@@ -2499,7 +2503,7 @@ check_transfer_statuses(LrDownload *dd, GError **err)
24992503
target->curl_handle = NULL;
25002504
g_free(target->headercb_interrupt_reason);
25012505
target->headercb_interrupt_reason = NULL;
2502-
cleanup_transcode(target, err);
2506+
cleanup_transcode(target, NULL);
25032507
fclose(target->f);
25042508
target->f = NULL;
25052509
if (target->curl_rqheaders) {
@@ -2903,7 +2907,7 @@ lr_download(GSList *targets,
29032907
curl_multi_remove_handle(dd.multi_handle, target->curl_handle);
29042908
curl_easy_cleanup(target->curl_handle);
29052909
target->curl_handle = NULL;
2906-
cleanup_transcode(target, err);
2910+
cleanup_transcode(target, NULL);
29072911
fclose(target->f);
29082912
target->f = NULL;
29092913
g_free(target->headercb_interrupt_reason);

0 commit comments

Comments
 (0)