Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 39 additions & 0 deletions inc/managers/class-membership-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ public function async_publish_pending_site($membership_id) {
/**
* Processes a delayed site publish action.
*
* Checks whether the pending site has been created. If is_publishing
* has been true for longer than 5 minutes, we assume the creation
* process was killed (PHP timeout, OOM, server restart) and reset
* the flag so the Action Scheduler retry can pick it up.
*
* @since 2.0.11
*/
public function check_pending_site_created() {
Expand All @@ -190,6 +195,40 @@ public function check_pending_site_created() {
exit;
}

/*
* Detect stale publishing state. When the PHP process that was
* creating the site gets killed mid-execution, is_publishing
* stays true forever — the AS retry sees the flag and bails
* out, creating an infinite loop. Reset the flag after 5 min
* so the next AS run or cron kick can retry site creation.
*
* @since 2.5.3
*/
if ($pending_site->is_publishing_stale()) {
$pending_site->set_publishing(false);

$membership->update_pending_site($pending_site);

wu_log_add(
self::LOG_FILE_NAME,
sprintf(
// translators: %d: membership ID.
__('Reset stale is_publishing flag for membership %d. The site creation process appears to have been killed before completing.', 'ultimate-multisite'),
$membership->get_id()
)
);

/*
* Re-enqueue the async action so Action Scheduler retries
* the site creation without waiting for the next cron tick.
*/
wu_enqueue_async_action('wu_async_publish_pending_site', ['membership_id' => $membership->get_id()], 'membership');

wp_send_json(['publish_status' => 'stopped']);
Comment on lines +221 to +227
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid starting two retries from the stale branch.

This path already reports publish_status => 'stopped', and the PR contract says the frontend retries from that state. Enqueuing wu_async_publish_pending_site here means stale recovery can race a frontend retry against an Action Scheduler retry, while Membership::publish_pending_site() still only has the serialized is_publishing flag as a coarse guard. Pick one retry source here.

🔧 Suggested change
-			/*
-			 * Re-enqueue the async action so Action Scheduler retries
-			 * the site creation without waiting for the next cron tick.
-			 */
-			wu_enqueue_async_action('wu_async_publish_pending_site', ['membership_id' => $membership->get_id()], 'membership');
-
 			wp_send_json(['publish_status' => 'stopped']);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/*
* Re-enqueue the async action so Action Scheduler retries
* the site creation without waiting for the next cron tick.
*/
wu_enqueue_async_action('wu_async_publish_pending_site', ['membership_id' => $membership->get_id()], 'membership');
wp_send_json(['publish_status' => 'stopped']);
wp_send_json(['publish_status' => 'stopped']);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@inc/managers/class-membership-manager.php` around lines 221 - 227, The code
currently enqueues wu_enqueue_async_action('wu_async_publish_pending_site', ...)
and then returns wp_send_json(['publish_status' => 'stopped']), creating two
competing retry sources; remove the Action Scheduler enqueue here so only the
frontend retry (publish_status => 'stopped') is relied on, or alternatively gate
the enqueue behind a check that ensures Membership::publish_pending_site() won't
be retried by both sources (e.g., consult the serialized is_publishing flag or a
new marker before calling wu_enqueue_async_action). Update the block containing
wu_enqueue_async_action and wp_send_json to either drop the enqueue call
entirely or add the guard so only a single retry path is used.


exit;
}

wp_send_json(['publish_status' => $pending_site->is_publishing() ? 'running' : 'stopped']);

exit;
Expand Down
66 changes: 65 additions & 1 deletion inc/models/class-site.php
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,18 @@ class Site extends Base_Model implements Limitable, Notable {
*/
protected $is_publishing;

/**
* Unix timestamp when is_publishing was set to true.
*
* Used to detect stale publishing state (e.g. process killed mid-creation).
* Not persisted to the sites DB table — only lives in the serialized
* pending_site metadata on memberships.
*
* @since 2.5.3
* @var int
*/
protected $publishing_started_at = 0;

/**
* Is this a active site?
*
Expand Down Expand Up @@ -753,13 +765,65 @@ public function is_publishing() {
/**
* Set if the site is being published.
*
* When set to true, also records the current timestamp so that
* stale publishing states can be detected and reset.
*
* @since 2.0.11
* @param int $publishing Holds the ID of the customer that owns this site.
* @param bool $publishing Whether the site is currently being published.
* @return void
*/
public function set_publishing($publishing): void {

$this->is_publishing = $publishing;

if ($publishing) {
$this->publishing_started_at = time();
} else {
$this->publishing_started_at = 0;
}
}

/**
* Get the Unix timestamp when publishing started.
*
* @since 2.5.3
* @return int Unix timestamp, or 0 if not publishing.
*/
public function get_publishing_started_at() {

return (int) $this->publishing_started_at;
}

/**
* Check if the publishing state is stale (stuck).
*
* A publishing state is considered stale if is_publishing has been
* true for longer than the given timeout. This happens when the PHP
* process is killed mid-creation (OOM, timeout, server restart)
* after the flag is set but before the error handlers can reset it.
*
* @since 2.5.3
* @param int $timeout_seconds Maximum allowed publishing duration in seconds. Default 300 (5 minutes).
* @return bool True if publishing started more than $timeout_seconds ago.
*/
public function is_publishing_stale($timeout_seconds = 300) {

if ( ! $this->is_publishing()) {
return false;
}

$started = $this->get_publishing_started_at();

/*
* If publishing_started_at is 0 or missing (pre-2.5.3 serialized
* objects), treat the state as stale — there's no way to know
* when it started, and the process that set it is clearly gone.
*/
if (empty($started)) {
return true;
}

return (time() - $started) > $timeout_seconds;
}

/**
Expand Down
83 changes: 83 additions & 0 deletions tests/WP_Ultimo/Models/Site_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,89 @@ public function test_publishing_false(): void {
$this->assertFalse($this->site->is_publishing(), 'is_publishing should return false after set_publishing(false).');
}

/**
* Test set_publishing(true) records publishing_started_at timestamp.
*/
public function test_set_publishing_records_timestamp(): void {
$before = time();
$this->site->set_publishing(true);
$after = time();

$started = $this->site->get_publishing_started_at();
$this->assertGreaterThanOrEqual($before, $started);
$this->assertLessThanOrEqual($after, $started);
}

/**
* Test set_publishing(false) clears publishing_started_at.
*/
public function test_set_publishing_false_clears_timestamp(): void {
$this->site->set_publishing(true);
$this->assertGreaterThan(0, $this->site->get_publishing_started_at());

$this->site->set_publishing(false);
$this->assertEquals(0, $this->site->get_publishing_started_at());
}

/**
* Test is_publishing_stale returns false when not publishing.
*/
public function test_is_publishing_stale_when_not_publishing(): void {
$this->site->set_publishing(false);
$this->assertFalse($this->site->is_publishing_stale());
}

/**
* Test is_publishing_stale returns false when recently started.
*/
public function test_is_publishing_stale_when_recent(): void {
$this->site->set_publishing(true);
$this->assertFalse($this->site->is_publishing_stale(), 'Should not be stale immediately after starting.');
}

/**
* Test is_publishing_stale returns true after timeout expires.
*/
public function test_is_publishing_stale_after_timeout(): void {
$this->site->set_publishing(true);

// Simulate time passing by setting publishing_started_at to 10 minutes ago.
$reflection = new \ReflectionProperty(Site::class, 'publishing_started_at');
$reflection->setAccessible(true);
$reflection->setValue($this->site, time() - 600);

$this->assertTrue($this->site->is_publishing_stale(), 'Should be stale after 10 minutes (default timeout 300s).');
}

/**
* Test is_publishing_stale with custom timeout.
*/
public function test_is_publishing_stale_custom_timeout(): void {
$this->site->set_publishing(true);

$reflection = new \ReflectionProperty(Site::class, 'publishing_started_at');
$reflection->setAccessible(true);
$reflection->setValue($this->site, time() - 60);

$this->assertFalse($this->site->is_publishing_stale(120), 'Should not be stale with 120s timeout after 60s.');
$this->assertTrue($this->site->is_publishing_stale(30), 'Should be stale with 30s timeout after 60s.');
}

/**
* Test is_publishing_stale returns true for pre-2.5.3 objects without timestamp.
*/
public function test_is_publishing_stale_legacy_object_without_timestamp(): void {
$this->site->set_publishing(true);

// Simulate a pre-2.5.3 serialized object that has is_publishing=true
// but no publishing_started_at value.
$reflection = new \ReflectionProperty(Site::class, 'publishing_started_at');
$reflection->setAccessible(true);
$reflection->setValue($this->site, 0);

$this->assertTrue($this->site->is_publishing_stale(), 'Pre-2.5.3 objects without timestamp should be treated as stale.');
}

/**
* Test get_template returns false when template_id does not match a site.
*/
Expand Down
Loading