diff --git a/changes/bug24086 b/changes/bug24086
new file mode 100644
index 0000000000..2ae0b37e65
--- /dev/null
+++ b/changes/bug24086
@@ -0,0 +1,7 @@
+ o Minor bugfixes (directory cache):
+ - When a consensus diff calculation is only partially successful, only
+ record the successful parts as having succeeded. Partial success
+ can happen if (for example) one compression method fails but
+ the others succeed. Previously we misrecorded all the calculations as
+ having succeeded, which would later cause a nonfatal assertion failure.
+ Fixes bug 24086; bugfix on 0.3.1.1-alpha.
diff --git a/changes/bug24099 b/changes/bug24099
new file mode 100644
index 0000000000..dca3992664
--- /dev/null
+++ b/changes/bug24099
@@ -0,0 +1,4 @@
+ o Minor bugfixes (directory cache):
+ - Recover better from empty or corrupt files in the consensus cache
+ directory. Fixes bug 24099; bugfix on 0.3.1.1-alpha.
+
diff --git a/src/common/storagedir.c b/src/common/storagedir.c
index 31933f64c2..c471ea911f 100644
--- a/src/common/storagedir.c
+++ b/src/common/storagedir.c
@@ -187,14 +187,19 @@ storage_dir_get_usage(storage_dir_t *d)
return d->usage;
}
-/** Mmap a specified file within d. */
+/** Mmap a specified file within d.
+ *
+ * On failure, return NULL and set errno as for tor_mmap_file(). */
tor_mmap_t *
storage_dir_map(storage_dir_t *d, const char *fname)
{
char *path = NULL;
tor_asprintf(&path, "%s/%s", d->directory, fname);
tor_mmap_t *result = tor_mmap_file(path);
+ int errval = errno;
tor_free(path);
+ if (result == NULL)
+ errno = errval;
return result;
}
@@ -364,6 +369,10 @@ storage_dir_save_labeled_to_file(storage_dir_t *d,
* the data's size into *sz_out. On success, also return a tor_mmap_t
* object whose contents should not be used -- it needs to be kept around,
* though, for as long as data_out is going to be valid.
+ *
+ * On failure, set errno as for tor_mmap_file() if the file was missing or
+ * empty, and set errno to EINVAL if the file was not in the labeled
+ * format expected.
*/
tor_mmap_t *
storage_dir_map_labeled(storage_dir_t *dir,
@@ -373,13 +382,20 @@ storage_dir_map_labeled(storage_dir_t *dir,
size_t *sz_out)
{
tor_mmap_t *m = storage_dir_map(dir, fname);
- if (! m)
+ int errval;
+ if (! m) {
+ errval = errno;
goto err;
+ }
const char *nulp = memchr(m->data, '\0', m->size);
- if (! nulp)
+ if (! nulp) {
+ errval = EINVAL;
goto err;
- if (labels_out && config_get_lines(m->data, labels_out, 0) < 0)
+ }
+ if (labels_out && config_get_lines(m->data, labels_out, 0) < 0) {
+ errval = EINVAL;
goto err;
+ }
size_t offset = nulp - m->data + 1;
tor_assert(offset <= m->size);
*data_out = (const uint8_t *)(m->data + offset);
@@ -388,6 +404,7 @@ storage_dir_map_labeled(storage_dir_t *dir,
return m;
err:
tor_munmap_file(m);
+ errno = errval;
return NULL;
}
diff --git a/src/or/conscache.c b/src/or/conscache.c
index 0f3e453eaf..1a15126f97 100644
--- a/src/or/conscache.c
+++ b/src/or/conscache.c
@@ -539,9 +539,20 @@ consensus_cache_rescan(consensus_cache_t *cache)
map = storage_dir_map_labeled(cache->dir, fname,
&labels, &body, &bodylen);
if (! map) {
- /* Can't load this; continue */
- log_warn(LD_FS, "Unable to map file %s from consensus cache: %s",
- escaped(fname), strerror(errno));
+ /* The ERANGE error might come from tor_mmap_file() -- it means the file
+ * was empty. EINVAL might come from ..map_labeled() -- it means the
+ * file was misformatted. In both cases, we should just delete it.
+ */
+ if (errno == ERANGE || errno == EINVAL) {
+ log_warn(LD_FS, "Found %s file %s in consensus cache; removing it.",
+ errno == ERANGE ? "empty" : "misformatted",
+ escaped(fname));
+ storage_dir_remove_file(cache->dir, fname);
+ } else {
+ /* Can't load this; continue */
+ log_warn(LD_FS, "Unable to map file %s from consensus cache: %s",
+ escaped(fname), strerror(errno));
+ }
continue;
}
consensus_cache_entry_t *ent =
diff --git a/src/or/consdiffmgr.c b/src/or/consdiffmgr.c
index 1d63d59577..e539b61484 100644
--- a/src/or/consdiffmgr.c
+++ b/src/or/consdiffmgr.c
@@ -283,6 +283,10 @@ cdm_diff_ht_set_status(consensus_flavor_t flav,
int status,
consensus_cache_entry_handle_t *handle)
{
+ if (handle == NULL) {
+ tor_assert_nonfatal(status != CDM_DIFF_PRESENT);
+ }
+
struct cdm_diff_t search, *ent;
memset(&search, 0, sizeof(cdm_diff_t));
search.flavor = flav;
@@ -1589,8 +1593,13 @@ consensus_diff_worker_replyfn(void *work_)
for (u = 0; u < ARRAY_LENGTH(handles); ++u) {
compress_method_t method = compress_diffs_with[u];
if (cache) {
- cdm_diff_ht_set_status(flav, from_sha3, to_sha3, method, status,
- handles[u]);
+ consensus_cache_entry_handle_t *h = handles[u];
+ int this_status = status;
+ if (h == NULL) {
+ this_status = CDM_DIFF_ERROR;
+ }
+ tor_assert_nonfatal(h != NULL || this_status == CDM_DIFF_ERROR);
+ cdm_diff_ht_set_status(flav, from_sha3, to_sha3, method, this_status, h);
} else {
consensus_cache_entry_handle_free(handles[u]);
}