Skip to content

fix(backup): clean up staging/temp files on abnormal exit and tighten disk-space pre-check #482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mrrobot47 wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

fix(backup): clean up staging/temp files on abnormal exit and tighten disk-space pre-check #482

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8410991
fix(backup): clean up staging artifacts and tighten disk pre-check
mrrobot47 Jun 29, 2026
66e7ca0
fix(backup): harden cleanup ordering, shutdown safety and disk estimate
mrrobot47 Jun 29, 2026
e92c356
fix(backup): guard per-site lock removal and drop a redundant temp
mrrobot47 Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 177 additions & 19 deletions src/helper/Site_Backup_Restore.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,20 @@ class Site_Backup_Restore {
// Global backup lock handle for serializing backups
private $global_backup_lock_handle = null;

// Local staging dir (EE_BACKUP_DIR/<site_url>) holding the in-progress archive
// (backup) or the downloaded archive (restore). Cleared once the success path
// removes it; the shutdown handler purges it only when an error/crash left it
// behind, so a failed run can't fill the disk or leave a stale partial download.
private $staging_dir = null;

// Temp SQL query files written into the live web root, run, then removed. Each
// is removed inline on the success path; the shutdown handler is a safety net
// for an interrupt between write and remove.
private $temp_query_files = [];

public function __construct() {
$this->fs = new Filesystem();
register_shutdown_function( [ $this, 'cleanup_temp_query_files' ] );
}

public function backup( $args, $assoc_args = [] ) {
Expand Down Expand Up @@ -127,6 +139,11 @@ public function backup( $args, $assoc_args = [] ) {
$this->pre_backup_check();
$backup_dir = EE_BACKUP_DIR . '/' . $this->site_data['site_url'];

// Track the staging dir so an abnormal exit (error, crash, OOM, Ctrl-C)
// purges the half-built archive instead of leaving it to fill the disk.
$this->staging_dir = $backup_dir;
register_shutdown_function( [ $this, 'cleanup_staging_dir' ] );

$this->fs->remove( $backup_dir );
$this->fs->mkdir( $backup_dir );

Expand All @@ -151,6 +168,7 @@ public function backup( $args, $assoc_args = [] ) {

$this->rclone_upload( $backup_dir );
$this->fs->remove( $backup_dir );
$this->staging_dir = null; // Removed cleanly; nothing for the shutdown handler to do.

$this->fs->remove( EE_BACKUP_DIR . '/' . $this->site_data['site_url'] . '.lock' );

Expand Down Expand Up @@ -265,10 +283,6 @@ public function restore( $args, $assoc_args = [] ) {
$backup_id = \EE\Utils\get_flag_value( $assoc_args, 'id' );
$backup_dir = EE_BACKUP_DIR . '/' . $this->site_data['site_url'];

if ( ! $this->fs->exists( $backup_dir ) ) {
$this->fs->mkdir( $backup_dir );
}

if ( $backup_id ) {

// verify_backup_id() lists remote backups (rclone lsf) before the
Expand All @@ -286,6 +300,17 @@ public function restore( $args, $assoc_args = [] ) {

$this->pre_restore_check();

// Track the staging dir for shutdown cleanup only AFTER pre_restore_check()
// has acquired this site's lock. Setting it earlier would let an early exit
// (e.g. invalid backup id, lock held by another process) delete a dir that a
// concurrent backup/restore of the same site is actively writing into.
$this->staging_dir = $backup_dir;
register_shutdown_function( [ $this, 'cleanup_staging_dir' ] );

if ( ! $this->fs->exists( $backup_dir ) ) {
$this->fs->mkdir( $backup_dir );
}

if ( 'wp' === $this->site_data['site_type'] ) {
$this->restore_wp( $backup_dir );
} else {
Expand All @@ -296,6 +321,7 @@ public function restore( $args, $assoc_args = [] ) {
$this->maybe_restore_custom_docker_compose( $backup_dir );

$this->fs->remove( $backup_dir );
$this->staging_dir = null; // Removed cleanly; nothing for the shutdown handler to do.

EE::log( 'Reloading site.' );
EE::run_command( [ 'site', 'reload', $this->site_data['site_url'] ], [], [] );
Expand Down Expand Up @@ -342,10 +368,13 @@ private function backup_site_details( $backup_dir ) {

$query = 'SELECT COUNT(*) FROM ' . $table_prefix . 'posts WHERE post_type = "attachment"';
$query_file = EE_ROOT_DIR . '/sites/' . $this->site_data['site_url'] . '/app/htdocs/query.sql';
// Track for shutdown cleanup in case an interrupt hits before the remove below.
$this->temp_query_files[] = $query_file;
$this->fs->dumpFile( $query_file, $query );
$upload_count = $this->run_wp_cli_command( 'db query < /var/www/htdocs/query.sql --skip-column-names | tr -d \'[:space:]\'', true );
$upload_count = empty( $upload_count ) ? 0 : $upload_count;
$this->fs->remove( $query_file );
$this->temp_query_files = array_values( array_diff( $this->temp_query_files, [ $query_file ] ) );

$plugin_count = $this->run_wp_cli_command( 'plugin list --format=count' );
// if it is not a number, then make it -
Expand Down Expand Up @@ -961,26 +990,61 @@ private function pre_backup_restore_checks() {
private function pre_backup_check() {
$this->pre_backup_restore_checks();

$site_path = EE_ROOT_DIR . '/sites/' . $this->site_data['site_url'] . '/app/htdocs';
$site_size = $this->dir_size( $site_path );
$site_root = EE_ROOT_DIR . '/sites/' . $this->site_data['site_url'];

EE::debug( 'Site size: ' . $site_size );
if ( ! $this->fs->exists( $site_root . '/app' ) ) {
$this->capture_error(
sprintf( 'Site app directory not found: %s/app', $site_root ),
self::ERROR_TYPE_FILESYSTEM,
3003
);
$this->fs->remove( EE_BACKUP_DIR . '/' . $this->site_data['site_url'] . '.lock' );
EE::error( "Site app directory does not exist: $site_root/app" );

@mrrobot47 mrrobot47 Jun 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e92c356 — per-site lock removals now go through a best-effort try_remove_site_lock() (try/catch), so a Filesystem::remove() failure on an error path can no longer mask the intended EE::error().

}

// Size everything actually archived, not just app/htdocs: every site type
// archives the whole app/ dir, and all types also archive config/ (nginx
// for all, php for php/wp). The previous estimate sized only app/htdocs and
// so missed the rest of app/ and the config archive entirely.
$db_size = 0;
$site_size = $this->dir_size( $site_root . '/app' );
$site_size += $this->dir_size( $site_root . '/config' );

EE::debug( 'Site size (files): ' . $site_size );

@mrrobot47 mrrobot47 Jun 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e92c356 — inlined the single-use $db_size directly into $site_size.


if ( in_array( $this->site_data['site_type'], [ 'php', 'wp' ] ) && ! empty( $this->site_data['db_name'] ) ) {
$site_size += $this->get_db_size();
$db_size = $this->get_db_size();
$site_size += $db_size;
EE::debug( 'Site size with db: ' . $site_size );
}

// Require free space >= the full uncompressed source ($site_size already
// includes the DB) plus 10% slack. The compressed archive is bounded by the
// source (7z output <= input), and the transient uncompressed SQL dump is a
// copy of the DB that already lives in $site_size, so a flat 10% headroom
// for filesystem slack / archive overhead is enough -- without re-adding the
// DB a second time, which would false-reject DB-heavy sites.
$required_size = (int) ceil( $site_size * 1.1 );

$free_space = disk_free_space( EE_BACKUP_DIR );
EE::debug( 'Free space: ' . $free_space );
if ( false === $free_space ) {
$this->capture_error(
'Unable to determine free disk space for backup directory',
self::ERROR_TYPE_FILESYSTEM,
3004
);
$this->fs->remove( EE_BACKUP_DIR . '/' . $this->site_data['site_url'] . '.lock' );
EE::error( 'Unable to determine free disk space for backup directory.' );

@mrrobot47 mrrobot47 Jun 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e92c356 — per-site lock removals now go through a best-effort try_remove_site_lock() (try/catch), so a Filesystem::remove() failure on an error path can no longer mask the intended EE::error().

}
EE::debug( 'Required space (with headroom): ' . $required_size . ', Free space: ' . $free_space );

if ( $site_size > $free_space ) {
$error_message = $this->build_disk_space_error_message( 'backup', $site_size, $free_space );
if ( $required_size > $free_space ) {
$error_message = $this->build_disk_space_error_message( 'backup', $required_size, $free_space );

$this->capture_error(
sprintf(
'Insufficient disk space for backup. Required: %s, Available: %s',
$this->format_bytes( $site_size ),
$this->format_bytes( $required_size ),
$this->format_bytes( $free_space )
),
self::ERROR_TYPE_DISK_SPACE,
Expand Down Expand Up @@ -1160,22 +1224,48 @@ private function format_bytes( $bytes, $precision = 2 ) {
return round( $size, $precision ) . ' ' . $units[ $pow ];
}

/**
* Sum the byte size of every file under $directory.
*
* A missing directory returns 0 (some archived dirs are optional, e.g. a
* site's config/ may not exist yet) rather than aborting. Files whose size
* can't be read are counted as 0 but raise a warning so the disk-space
* pre-check isn't silently under-estimating. CATCH_GET_CHILD makes an
* untraversable subdirectory (e.g. permission denied) be skipped rather than
* throw UnexpectedValueException and abort the whole backup; such a subtree is
* simply not counted (the estimate may then be low).
*/
private function dir_size( string $directory ) {
$size = 0;

EE::debug( "Calculating size of $directory" );

if ( ! $this->fs->exists( $directory ) ) {
EE::error( "Directory does not exist: $directory" );
return 0;
}

$files = new \RecursiveIteratorIterator( new \RecursiveDirectoryIterator( $directory, \FilesystemIterator::SKIP_DOTS ) );
$files = new \RecursiveIteratorIterator(
new \RecursiveDirectoryIterator( $directory, \FilesystemIterator::SKIP_DOTS ),
\RecursiveIteratorIterator::LEAVES_ONLY,
\RecursiveIteratorIterator::CATCH_GET_CHILD
);
$unreadable = 0;

foreach ( $files as $file ) {
if ( ! $file->isReadable() ) {
$unreadable++;
continue;
}
$file_size = $file->getSize();
if ( false === $file_size ) {
$unreadable++;
continue;
}
$size += $file->getSize();
$size += $file_size;
}

if ( $unreadable > 0 ) {
EE::warning( sprintf( 'Could not size %d file(s) under %s; disk-space estimate may be low.', $unreadable, $directory ) );
}

EE::debug( "Size of $directory: $size" );
Expand Down Expand Up @@ -1204,6 +1294,8 @@ private function get_db_size() {


$query_file = EE_ROOT_DIR . '/sites/' . $this->site_data['site_url'] . '/app/htdocs/db_size_query.sql';
// Track for shutdown cleanup in case an interrupt hits before the remove below.
$this->temp_query_files[] = $query_file;
$this->fs->dumpFile( $query_file, $query );


Expand All @@ -1213,14 +1305,25 @@ private function get_db_size() {


$this->fs->remove( $query_file );
$this->temp_query_files = array_values( array_diff( $this->temp_query_files, [ $query_file ] ) );


$size = 0;
$size_found = false;
$size_output = explode( "\n", $output->stdout );

if ( count( $size_output ) > 1 ) {
$size_array = explode( "\t", $size_output[1] );
$size = isset( $size_array[1] ) ? $size_array[1] : 0;
if ( isset( $size_array[1] ) && is_numeric( $size_array[1] ) ) {
$size = $size_array[1];
$size_found = true;
}
}

// A backup proceeds even if the DB size can't be read, but warn so the
// disk-space pre-check isn't silently treating an unknown DB as 0 bytes.
if ( ! $size_found ) {
EE::warning( 'Could not determine database size; disk-space estimate may be low.' );
}

EE::debug( "DB size: $size" );
Expand All @@ -1241,7 +1344,10 @@ private function list_remote_backups( $return = false ) {
return [];
}

$backups = explode( PHP_EOL, trim( $output->stdout ) ); // Remove extra whitespace and split
// trim()+explode() on empty output yields [''] (one empty element), so
// filter blank lines before the empty() check below -- otherwise the
// "No remote backups found" branch is unreachable for an empty listing.
$backups = array_filter( array_map( 'trim', explode( PHP_EOL, $output->stdout ) ), 'strlen' );

if ( empty( $backups ) ) {
if ( ! $return ) {
Expand Down Expand Up @@ -1598,8 +1704,10 @@ private function cleanup_old_backups() {
return;
}

// Check if we have more backups than allowed
if ( count( $backups ) > ( $no_of_backups + 1 ) ) {
// Check if we have more backups than allowed. array_slice() below keeps the
// first $no_of_backups, so trigger as soon as the count exceeds that (the
// previous `+ 1` retained N+1).
if ( count( $backups ) > $no_of_backups ) {
$backups_to_delete = array_slice( $backups, $no_of_backups );

EE::log( sprintf( 'Cleaning up old backups. Keeping %d most recent backups.', $no_of_backups ) );
Expand Down Expand Up @@ -1963,4 +2071,54 @@ public function release_global_backup_lock() {
EE::debug( 'Released global backup lock' );
}
}

/**
* Remove the local staging dir on abnormal exit.
*
* The success path removes it and clears $this->staging_dir, so this only
* fires when an error/crash left a half-built archive (backup) or a partial
* download (restore) behind. Idempotent and a no-op once cleared.
*
* @return void
*/
public function cleanup_staging_dir() {
// Never throw from a shutdown safety net: Filesystem::remove() raises
// IOException on a failed unlink (e.g. permission denied), and a throw here
// would abort the remaining shutdown handlers (lock release, dash callback).
try {
if ( ! empty( $this->staging_dir ) && $this->fs->exists( $this->staging_dir ) ) {
$this->fs->remove( $this->staging_dir );
EE::debug( 'Cleaned up staging dir after abnormal exit: ' . $this->staging_dir );
}
} catch ( \Throwable $e ) {
EE::debug( 'Could not clean up staging dir ' . $this->staging_dir . ': ' . $e->getMessage() );
}
$this->staging_dir = null;
}

/**
* Remove any temp SQL query files written into the live web root.
*
* Safety net for an interrupt between a query file being written and its
* inline removal; the success path removes them and clears the list, so this
* is normally a no-op. Idempotent.
*
* @return void
*/
public function cleanup_temp_query_files() {
// First-registered shutdown handler: must never throw, or the remaining
// handlers (staging cleanup, lock release, dash callback) would be skipped.
// Filesystem::remove() raises IOException on a failed unlink.
foreach ( $this->temp_query_files as $query_file ) {
try {
if ( $this->fs->exists( $query_file ) ) {
$this->fs->remove( $query_file );
EE::debug( 'Cleaned up leftover query file: ' . $query_file );
}
} catch ( \Throwable $e ) {
EE::debug( 'Could not clean up query file ' . $query_file . ': ' . $e->getMessage() );
}
}
$this->temp_query_files = [];
}
}
Loading