fix: free the parked delete manifest on every receiver error exit
The late/commit path keeps the received keep-set in a local list until STATUS_FINISHED. Error exits after it was parked (STATUS_ABORT, a failing receive_status / non-FINISHED status, a second manifest frame, or a later file/chunk/store failure) previously dropped the only reference and leaked up to ~16 MB of path strings + pointer array per connection. Both failure labels now discard the parked list exactly once; the successful FINISHED path still hands ownership to *pending_manifest (the -m caller) without freeing it.
This commit is contained in:
+37
-19
@@ -148,18 +148,21 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver
|
|||||||
if (!receive_status(file_descriptor, &status))
|
if (!receive_status(file_descriptor, &status))
|
||||||
return -1;
|
return -1;
|
||||||
bool early_delete = config_delete_timing_early(config);
|
bool early_delete = config_delete_timing_early(config);
|
||||||
|
/* Parked keep-set for the late/commit timing. Every exit path below frees it
|
||||||
|
exactly once; the only exception is the successful FINISHED handoff, which
|
||||||
|
transfers ownership to *pending_manifest (used by the -m receiver). */
|
||||||
ArrayList* deferred_manifest = NULL;
|
ArrayList* deferred_manifest = NULL;
|
||||||
while (status == STATUS_NEXT || status == STATUS_CHUNK || status == STATUS_CHECK ||
|
while (status == STATUS_NEXT || status == STATUS_CHUNK || status == STATUS_CHECK ||
|
||||||
status == STATUS_KEEPALIVE || status == STATUS_ABORT || status == STATUS_CHECK_BATCH ||
|
status == STATUS_KEEPALIVE || status == STATUS_ABORT || status == STATUS_CHECK_BATCH ||
|
||||||
status == STATUS_MKDIR || status == STATUS_MANIFEST) {
|
status == STATUS_MKDIR || status == STATUS_MANIFEST) {
|
||||||
if (status == STATUS_KEEPALIVE) {
|
if (status == STATUS_KEEPALIVE) {
|
||||||
if (!send_status(file_descriptor, STATUS_KEEPALIVE))
|
if (!send_status(file_descriptor, STATUS_KEEPALIVE))
|
||||||
return -1;
|
goto fail;
|
||||||
goto next_status;
|
goto next_status;
|
||||||
}
|
}
|
||||||
if (status == STATUS_ABORT) {
|
if (status == STATUS_ABORT) {
|
||||||
log_message(LOG_LEVEL_INFO, "Received abort from client, cleaning up");
|
log_message(LOG_LEVEL_INFO, "Received abort from client, cleaning up");
|
||||||
return -1;
|
goto fail;
|
||||||
}
|
}
|
||||||
if (status == STATUS_CHECK) {
|
if (status == STATUS_CHECK) {
|
||||||
bool skipped;
|
bool skipped;
|
||||||
@@ -172,7 +175,7 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver
|
|||||||
goto receive_error;
|
goto receive_error;
|
||||||
} else if (status == STATUS_CHECK_BATCH) {
|
} else if (status == STATUS_CHECK_BATCH) {
|
||||||
if (!receiver_process_batch(config, file_descriptor))
|
if (!receiver_process_batch(config, file_descriptor))
|
||||||
return -1;
|
goto fail;
|
||||||
goto next_status;
|
goto next_status;
|
||||||
} else if (status == STATUS_MKDIR) {
|
} else if (status == STATUS_MKDIR) {
|
||||||
File* dir = file_receive_directory(file_descriptor);
|
File* dir = file_receive_directory(file_descriptor);
|
||||||
@@ -181,7 +184,7 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver
|
|||||||
} else if (status == STATUS_MANIFEST) {
|
} else if (status == STATUS_MANIFEST) {
|
||||||
ArrayList* manifest = receive_manifest_entries(file_descriptor);
|
ArrayList* manifest = receive_manifest_entries(file_descriptor);
|
||||||
if (!manifest)
|
if (!manifest)
|
||||||
return -1; /* receive_manifest_entries already sent STATUS_ERROR */
|
goto fail; /* receive_manifest_entries already sent STATUS_ERROR */
|
||||||
if (early_delete) {
|
if (early_delete) {
|
||||||
/* --delete-before / --delete-during: the manifest is authoritative the
|
/* --delete-before / --delete-during: the manifest is authoritative the
|
||||||
moment it arrives, before any file data. Delete now and acknowledge
|
moment it arrives, before any file data. Delete now and acknowledge
|
||||||
@@ -192,24 +195,24 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver
|
|||||||
array_list_delete(manifest);
|
array_list_delete(manifest);
|
||||||
if (!deletion_ok) {
|
if (!deletion_ok) {
|
||||||
send_status(file_descriptor, STATUS_ERROR);
|
send_status(file_descriptor, STATUS_ERROR);
|
||||||
return -1;
|
goto fail;
|
||||||
}
|
}
|
||||||
if (!send_status(file_descriptor, STATUS_OK))
|
if (!send_status(file_descriptor, STATUS_OK))
|
||||||
return -1;
|
goto fail;
|
||||||
} else {
|
} else if (config->use_delete) {
|
||||||
/* Plain --delete / --delete-after / --delete-delay: hold the keep-set
|
/* Plain --delete / --delete-after / --delete-delay: hold the keep-set
|
||||||
and commit the deletion only after STATUS_FINISHED. */
|
and commit the deletion only after STATUS_FINISHED. */
|
||||||
if (config->use_delete) {
|
if (deferred_manifest) {
|
||||||
if (deferred_manifest) {
|
log_message(LOG_LEVEL_ERROR, "Received a second delete manifest");
|
||||||
log_message(LOG_LEVEL_ERROR, "Received a second delete manifest");
|
array_list_delete(deferred_manifest);
|
||||||
array_list_delete(manifest);
|
deferred_manifest = NULL;
|
||||||
send_status(file_descriptor, STATUS_ERROR);
|
|
||||||
return -1;
|
|
||||||
}
|
|
||||||
deferred_manifest = manifest;
|
|
||||||
} else {
|
|
||||||
array_list_delete(manifest);
|
array_list_delete(manifest);
|
||||||
|
send_status(file_descriptor, STATUS_ERROR);
|
||||||
|
goto fail;
|
||||||
}
|
}
|
||||||
|
deferred_manifest = manifest;
|
||||||
|
} else {
|
||||||
|
array_list_delete(manifest);
|
||||||
}
|
}
|
||||||
goto next_status;
|
goto next_status;
|
||||||
} else {
|
} else {
|
||||||
@@ -241,26 +244,41 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver
|
|||||||
if (deferred_manifest) {
|
if (deferred_manifest) {
|
||||||
if (pending_manifest) {
|
if (pending_manifest) {
|
||||||
*pending_manifest = deferred_manifest;
|
*pending_manifest = deferred_manifest;
|
||||||
|
deferred_manifest = NULL;
|
||||||
} else {
|
} else {
|
||||||
bool deletion_ok = manifest_delete_extras(config, deferred_manifest);
|
bool deletion_ok = manifest_delete_extras(config, deferred_manifest);
|
||||||
array_list_delete(deferred_manifest);
|
array_list_delete(deferred_manifest);
|
||||||
|
deferred_manifest = NULL;
|
||||||
if (!deletion_ok) {
|
if (!deletion_ok) {
|
||||||
send_status(file_descriptor, STATUS_ERROR);
|
send_status(file_descriptor, STATUS_ERROR);
|
||||||
return -1;
|
goto fail;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (sink->send_success) {
|
if (sink->send_success) {
|
||||||
if (sink->send_success_frame) {
|
if (sink->send_success_frame) {
|
||||||
if (!sink->send_success_frame(file_descriptor, sink->context))
|
if (!sink->send_success_frame(file_descriptor, sink->context))
|
||||||
return -1;
|
goto fail;
|
||||||
} else if (!send_status(file_descriptor, STATUS_OK)) {
|
} else if (!send_status(file_descriptor, STATUS_OK)) {
|
||||||
return -1;
|
goto fail;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
|
fail:
|
||||||
|
/* Failure exits that must not (or already did) report a STATUS_ERROR. The
|
||||||
|
parked keep-set is dropped: never commit a deletion for a failed stream. */
|
||||||
|
if (deferred_manifest) {
|
||||||
|
array_list_delete(deferred_manifest);
|
||||||
|
deferred_manifest = NULL;
|
||||||
|
}
|
||||||
|
return -1;
|
||||||
|
|
||||||
receive_error:
|
receive_error:
|
||||||
|
if (deferred_manifest) {
|
||||||
|
array_list_delete(deferred_manifest);
|
||||||
|
deferred_manifest = NULL;
|
||||||
|
}
|
||||||
if (sink->send_error)
|
if (sink->send_error)
|
||||||
send_status(file_descriptor, STATUS_ERROR);
|
send_status(file_descriptor, STATUS_ERROR);
|
||||||
return -1;
|
return -1;
|
||||||
|
|||||||
Reference in New Issue
Block a user