Skip to content

Commit 566400a

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 638dd0a commit 566400a

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
@@ -1530,8 +1530,12 @@ maybe_transcode(LrTarget *target, GError **err)
15301530
}
15311531

15321532
void
1533-
cleanup_transcode(LrTarget *target, GError **err)
1533+
cleanup_transcode(LrTarget *target, GError **transfer_err)
15341534
{
1535+
/** transfer_err can be NULL if we're using this to clean up a failed
1536+
* transfer. In that circumstance g_set_error does nothing which is fine,
1537+
* we don't need to pile on a second failure reason.
1538+
*/
15351539
int wstatus, trc;
15361540
if (!target->writef) {
15371541
return;
@@ -1541,21 +1545,21 @@ cleanup_transcode(LrTarget *target, GError **err)
15411545
}
15421546
fclose(target->writef);
15431547
if(waitpid(target->pid, &wstatus, 0) == -1) {
1544-
g_set_error(err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
1548+
g_set_error(transfer_err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
15451549
"transcode waitpid failed: %s", g_strerror(errno));
15461550
} else if (WIFEXITED(wstatus)) {
15471551
trc = WEXITSTATUS(wstatus);
15481552
if (trc != 0) {
1549-
g_set_error(err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
1553+
g_set_error(transfer_err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
15501554
"transcode process non-zero exit code %d", trc);
15511555
}
15521556
} else if (WIFSIGNALED(wstatus)) {
1553-
g_set_error(err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
1557+
g_set_error(transfer_err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
15541558
"transcode process was terminated with a signal: %d",
15551559
WTERMSIG(wstatus));
15561560
} else {
15571561
/* don't think this can happen, but covering all bases */
1558-
g_set_error(err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
1562+
g_set_error(transfer_err, LR_DOWNLOADER_ERROR, LRE_TRANSCODE,
15591563
"transcode unhandled circumstance in waitpid");
15601564
}
15611565
target->writef = NULL;
@@ -1822,7 +1826,7 @@ prepare_next_transfer(LrDownload *dd, gboolean *candidatefound, GError **err)
18221826
curl_easy_cleanup(target->curl_handle);
18231827
target->curl_handle = NULL;
18241828
}
1825-
cleanup_transcode(target, err);
1829+
cleanup_transcode(target, NULL);
18261830
if (target->f != NULL) {
18271831
fclose(target->f);
18281832
target->f = NULL;
@@ -2390,11 +2394,11 @@ check_transfer_statuses(LrDownload *dd, GError **err)
23902394
if (!ret) // Error
23912395
return FALSE;
23922396

2397+
cleanup_transcode(target, &transfer_err);
2398+
23932399
if (transfer_err) // Transfer was unsuccessful
23942400
goto transfer_error;
23952401

2396-
cleanup_transcode(target, err);
2397-
23982402
//
23992403
// Checksum checking
24002404
//
@@ -2489,7 +2493,7 @@ check_transfer_statuses(LrDownload *dd, GError **err)
24892493
target->curl_handle = NULL;
24902494
g_free(target->headercb_interrupt_reason);
24912495
target->headercb_interrupt_reason = NULL;
2492-
cleanup_transcode(target, err);
2496+
cleanup_transcode(target, NULL);
24932497
fclose(target->f);
24942498
target->f = NULL;
24952499
if (target->curl_rqheaders) {
@@ -2893,7 +2897,7 @@ lr_download(GSList *targets,
28932897
curl_multi_remove_handle(dd.multi_handle, target->curl_handle);
28942898
curl_easy_cleanup(target->curl_handle);
28952899
target->curl_handle = NULL;
2896-
cleanup_transcode(target, err);
2900+
cleanup_transcode(target, NULL);
28972901
fclose(target->f);
28982902
target->f = NULL;
28992903
g_free(target->headercb_interrupt_reason);

0 commit comments

Comments
 (0)