[6.x] Fix Stache index re-entrancy causing null URIs on cold stache

[o1y]

Fixesstatamic/seo-pro#508

Deep Stache internals ahead. Please try to break or optimize this.. I did my best 👀

On a cold stache, building an index calls getItem() for each entry, which hits getCachedItem(). That method then triggers ->resolveIndex('uri')->load() causing re-entrant index loading.

The issue: validateTree() sees incomplete indexes, removes all entries from the structure tree, and all URIs become null. The broken index gets cached, poisoning subsequent requests. That's why the sitemap.xml is empty or Entry::query()->whereNotNull('uri') returns null 🕳️

My introduced caching from #14031 also triggers this in single-site setups, since cached items cause getCachedItem() to be called during index builds.

The main change is about tracking nested index loads, instead of storing the currently loading index in a string. ☀️

-private static ?string $currentlyLoading = null;+private static array $loadingStack = [];

It seems also possible to just remove the URI blink from getCachedItem() (or just use the base class function), but this would break the performance optimization which was introduced in #9844

- if (! Index::isLoading() && $this->shouldBlinkEntryUris && ($uri = $this->resolveIndex('uri')->load()->get($entry->id()))) { - Blink::store('entry-uris')->put($entry->id(), $uri); - }

If possible, it might be worth backporting this to 5.x? 🤗

[mmodler]

@o1y Thanks for tracking this down!

We're running ~300 production instances on Statamic v5.73.11 and have been experiencing sporadic cases where all frontend pages return 404. A cache clear resolves the issue each time.

I came across this PR while investigating the root cause of these 404s. After analyzing the code, I'm fairly confident this is the same underlying issue — the affected code in Index.php and CollectionEntriesStore.php is identical between 5.x and 6.x, so this fix should be directly applicable to 5.x as well.

A backport to 5.x would be much appreciated, as this is actively affecting production instances.

[jasonvarga]

Thanks for the input @mmodler. We'd happily backport it.

Unless @o1y you could reopen this against 5.x?

[o1y]

Done ✅

[mmodler]

You guys rock!

[o1y]

Fixesstatamic/seo-pro#508

Deep Stache internals ahead. Please try to break or optimize this.. I did my best 👀

On a cold stache, building an index calls getItem() for each entry, which hits getCachedItem(). That method then triggers ->resolveIndex('uri')->load() causing re-entrant index loading.

The issue: validateTree() sees incomplete indexes, removes all entries from the structure tree, and all URIs become null. The broken index gets cached, poisoning subsequent requests. That's why the sitemap.xml is empty or Entry::query()->whereNotNull('uri') returns null 🕳️

My introduced caching from #14031 also triggers this in single-site setups, since cached items cause getCachedItem() to be called during index builds.

The main change is about tracking nested index loads, instead of storing the currently loading index in a string. ☀️

-private static ?string $currentlyLoading = null;+private static array $loadingStack = [];

It seems also possible to just remove the URI blink from getCachedItem() (or just use the base class function), but this would break the performance optimization which was introduced in #9844

- if (! Index::isLoading() && $this->shouldBlinkEntryUris && ($uri = $this->resolveIndex('uri')->load()->get($entry->id()))) { - Blink::store('entry-uris')->put($entry->id(), $uri); - }

If possible, it might be worth backporting this to 5.x? 🤗

[mmodler]

@o1y Thanks for tracking this down!

We're running ~300 production instances on Statamic v5.73.11 and have been experiencing sporadic cases where all frontend pages return 404. A cache clear resolves the issue each time.

I came across this PR while investigating the root cause of these 404s. After analyzing the code, I'm fairly confident this is the same underlying issue — the affected code in Index.php and CollectionEntriesStore.php is identical between 5.x and 6.x, so this fix should be directly applicable to 5.x as well.

A backport to 5.x would be much appreciated, as this is actively affecting production instances.

[jasonvarga]

Thanks for the input @mmodler. We'd happily backport it.

Unless @o1y you could reopen this against 5.x?

[o1y]

Done ✅

[mmodler]

You guys rock!

[o1y]

Fixesstatamic/seo-pro#508

Deep Stache internals ahead. Please try to break or optimize this.. I did my best 👀

On a cold stache, building an index calls getItem() for each entry, which hits getCachedItem(). That method then triggers ->resolveIndex('uri')->load() causing re-entrant index loading.

The issue: validateTree() sees incomplete indexes, removes all entries from the structure tree, and all URIs become null. The broken index gets cached, poisoning subsequent requests. That's why the sitemap.xml is empty or Entry::query()->whereNotNull('uri') returns null 🕳️

My introduced caching from #14031 also triggers this in single-site setups, since cached items cause getCachedItem() to be called during index builds.

The main change is about tracking nested index loads, instead of storing the currently loading index in a string. ☀️

-private static ?string $currentlyLoading = null;+private static array $loadingStack = [];

It seems also possible to just remove the URI blink from getCachedItem() (or just use the base class function), but this would break the performance optimization which was introduced in #9844

- if (! Index::isLoading() && $this->shouldBlinkEntryUris && ($uri = $this->resolveIndex('uri')->load()->get($entry->id()))) { - Blink::store('entry-uris')->put($entry->id(), $uri); - }

If possible, it might be worth backporting this to 5.x? 🤗

[mmodler]

@o1y Thanks for tracking this down!

We're running ~300 production instances on Statamic v5.73.11 and have been experiencing sporadic cases where all frontend pages return 404. A cache clear resolves the issue each time.

I came across this PR while investigating the root cause of these 404s. After analyzing the code, I'm fairly confident this is the same underlying issue — the affected code in Index.php and CollectionEntriesStore.php is identical between 5.x and 6.x, so this fix should be directly applicable to 5.x as well.

A backport to 5.x would be much appreciated, as this is actively affecting production instances.

[jasonvarga]

Thanks for the input @mmodler. We'd happily backport it.

Unless @o1y you could reopen this against 5.x?

[o1y]

Done ✅

[mmodler]

You guys rock!

[o1y]

Fixesstatamic/seo-pro#508

Deep Stache internals ahead. Please try to break or optimize this.. I did my best 👀

On a cold stache, building an index calls getItem() for each entry, which hits getCachedItem(). That method then triggers ->resolveIndex('uri')->load() causing re-entrant index loading.

The issue: validateTree() sees incomplete indexes, removes all entries from the structure tree, and all URIs become null. The broken index gets cached, poisoning subsequent requests. That's why the sitemap.xml is empty or Entry::query()->whereNotNull('uri') returns null 🕳️

My introduced caching from #14031 also triggers this in single-site setups, since cached items cause getCachedItem() to be called during index builds.

The main change is about tracking nested index loads, instead of storing the currently loading index in a string. ☀️

-private static ?string $currentlyLoading = null;+private static array $loadingStack = [];

It seems also possible to just remove the URI blink from getCachedItem() (or just use the base class function), but this would break the performance optimization which was introduced in #9844

- if (! Index::isLoading() && $this->shouldBlinkEntryUris && ($uri = $this->resolveIndex('uri')->load()->get($entry->id()))) { - Blink::store('entry-uris')->put($entry->id(), $uri); - }

If possible, it might be worth backporting this to 5.x? 🤗

[mmodler]

@o1y Thanks for tracking this down!

We're running ~300 production instances on Statamic v5.73.11 and have been experiencing sporadic cases where all frontend pages return 404. A cache clear resolves the issue each time.

I came across this PR while investigating the root cause of these 404s. After analyzing the code, I'm fairly confident this is the same underlying issue — the affected code in Index.php and CollectionEntriesStore.php is identical between 5.x and 6.x, so this fix should be directly applicable to 5.x as well.

A backport to 5.x would be much appreciated, as this is actively affecting production instances.

[jasonvarga]

Thanks for the input @mmodler. We'd happily backport it.

Unless @o1y you could reopen this against 5.x?

[o1y]

Done ✅

[mmodler]

You guys rock!

[o1y]

Fixesstatamic/seo-pro#508

Deep Stache internals ahead. Please try to break or optimize this.. I did my best 👀

On a cold stache, building an index calls getItem() for each entry, which hits getCachedItem(). That method then triggers ->resolveIndex('uri')->load() causing re-entrant index loading.

The issue: validateTree() sees incomplete indexes, removes all entries from the structure tree, and all URIs become null. The broken index gets cached, poisoning subsequent requests. That's why the sitemap.xml is empty or Entry::query()->whereNotNull('uri') returns null 🕳️

My introduced caching from #14031 also triggers this in single-site setups, since cached items cause getCachedItem() to be called during index builds.

The main change is about tracking nested index loads, instead of storing the currently loading index in a string. ☀️

-private static ?string $currentlyLoading = null;+private static array $loadingStack = [];

It seems also possible to just remove the URI blink from getCachedItem() (or just use the base class function), but this would break the performance optimization which was introduced in #9844

- if (! Index::isLoading() && $this->shouldBlinkEntryUris && ($uri = $this->resolveIndex('uri')->load()->get($entry->id()))) { - Blink::store('entry-uris')->put($entry->id(), $uri); - }

If possible, it might be worth backporting this to 5.x? 🤗

[mmodler]

@o1y Thanks for tracking this down!

We're running ~300 production instances on Statamic v5.73.11 and have been experiencing sporadic cases where all frontend pages return 404. A cache clear resolves the issue each time.

I came across this PR while investigating the root cause of these 404s. After analyzing the code, I'm fairly confident this is the same underlying issue — the affected code in Index.php and CollectionEntriesStore.php is identical between 5.x and 6.x, so this fix should be directly applicable to 5.x as well.

A backport to 5.x would be much appreciated, as this is actively affecting production instances.

[jasonvarga]

Thanks for the input @mmodler. We'd happily backport it.

Unless @o1y you could reopen this against 5.x?

[o1y]

Done ✅

[mmodler]

You guys rock!

[o1y]

Fixesstatamic/seo-pro#508

Deep Stache internals ahead. Please try to break or optimize this.. I did my best 👀

On a cold stache, building an index calls getItem() for each entry, which hits getCachedItem(). That method then triggers ->resolveIndex('uri')->load() causing re-entrant index loading.

The issue: validateTree() sees incomplete indexes, removes all entries from the structure tree, and all URIs become null. The broken index gets cached, poisoning subsequent requests. That's why the sitemap.xml is empty or Entry::query()->whereNotNull('uri') returns null 🕳️

My introduced caching from #14031 also triggers this in single-site setups, since cached items cause getCachedItem() to be called during index builds.

The main change is about tracking nested index loads, instead of storing the currently loading index in a string. ☀️

-private static ?string $currentlyLoading = null;+private static array $loadingStack = [];

It seems also possible to just remove the URI blink from getCachedItem() (or just use the base class function), but this would break the performance optimization which was introduced in #9844

- if (! Index::isLoading() && $this->shouldBlinkEntryUris && ($uri = $this->resolveIndex('uri')->load()->get($entry->id()))) { - Blink::store('entry-uris')->put($entry->id(), $uri); - }

If possible, it might be worth backporting this to 5.x? 🤗

[mmodler]

@o1y Thanks for tracking this down!

We're running ~300 production instances on Statamic v5.73.11 and have been experiencing sporadic cases where all frontend pages return 404. A cache clear resolves the issue each time.

I came across this PR while investigating the root cause of these 404s. After analyzing the code, I'm fairly confident this is the same underlying issue — the affected code in Index.php and CollectionEntriesStore.php is identical between 5.x and 6.x, so this fix should be directly applicable to 5.x as well.

A backport to 5.x would be much appreciated, as this is actively affecting production instances.

[jasonvarga]

Thanks for the input @mmodler. We'd happily backport it.

Unless @o1y you could reopen this against 5.x?

[o1y]

Done ✅

[mmodler]

You guys rock!

[o1y]

Fixesstatamic/seo-pro#508

Deep Stache internals ahead. Please try to break or optimize this.. I did my best 👀

On a cold stache, building an index calls getItem() for each entry, which hits getCachedItem(). That method then triggers ->resolveIndex('uri')->load() causing re-entrant index loading.

The issue: validateTree() sees incomplete indexes, removes all entries from the structure tree, and all URIs become null. The broken index gets cached, poisoning subsequent requests. That's why the sitemap.xml is empty or Entry::query()->whereNotNull('uri') returns null 🕳️

My introduced caching from #14031 also triggers this in single-site setups, since cached items cause getCachedItem() to be called during index builds.

The main change is about tracking nested index loads, instead of storing the currently loading index in a string. ☀️

-private static ?string $currentlyLoading = null;+private static array $loadingStack = [];

It seems also possible to just remove the URI blink from getCachedItem() (or just use the base class function), but this would break the performance optimization which was introduced in #9844

- if (! Index::isLoading() && $this->shouldBlinkEntryUris && ($uri = $this->resolveIndex('uri')->load()->get($entry->id()))) { - Blink::store('entry-uris')->put($entry->id(), $uri); - }

If possible, it might be worth backporting this to 5.x? 🤗

[mmodler]

@o1y Thanks for tracking this down!

We're running ~300 production instances on Statamic v5.73.11 and have been experiencing sporadic cases where all frontend pages return 404. A cache clear resolves the issue each time.

I came across this PR while investigating the root cause of these 404s. After analyzing the code, I'm fairly confident this is the same underlying issue — the affected code in Index.php and CollectionEntriesStore.php is identical between 5.x and 6.x, so this fix should be directly applicable to 5.x as well.

A backport to 5.x would be much appreciated, as this is actively affecting production instances.

[jasonvarga]

Thanks for the input @mmodler. We'd happily backport it.

Unless @o1y you could reopen this against 5.x?

[o1y]

Done ✅

[mmodler]

You guys rock!

[o1y]

Fixesstatamic/seo-pro#508

Deep Stache internals ahead. Please try to break or optimize this.. I did my best 👀

On a cold stache, building an index calls getItem() for each entry, which hits getCachedItem(). That method then triggers ->resolveIndex('uri')->load() causing re-entrant index loading.

The issue: validateTree() sees incomplete indexes, removes all entries from the structure tree, and all URIs become null. The broken index gets cached, poisoning subsequent requests. That's why the sitemap.xml is empty or Entry::query()->whereNotNull('uri') returns null 🕳️

My introduced caching from #14031 also triggers this in single-site setups, since cached items cause getCachedItem() to be called during index builds.

The main change is about tracking nested index loads, instead of storing the currently loading index in a string. ☀️

-private static ?string $currentlyLoading = null;+private static array $loadingStack = [];

It seems also possible to just remove the URI blink from getCachedItem() (or just use the base class function), but this would break the performance optimization which was introduced in #9844

- if (! Index::isLoading() && $this->shouldBlinkEntryUris && ($uri = $this->resolveIndex('uri')->load()->get($entry->id()))) { - Blink::store('entry-uris')->put($entry->id(), $uri); - }

If possible, it might be worth backporting this to 5.x? 🤗

[mmodler]

@o1y Thanks for tracking this down!

We're running ~300 production instances on Statamic v5.73.11 and have been experiencing sporadic cases where all frontend pages return 404. A cache clear resolves the issue each time.

I came across this PR while investigating the root cause of these 404s. After analyzing the code, I'm fairly confident this is the same underlying issue — the affected code in Index.php and CollectionEntriesStore.php is identical between 5.x and 6.x, so this fix should be directly applicable to 5.x as well.

A backport to 5.x would be much appreciated, as this is actively affecting production instances.

[jasonvarga]

Thanks for the input @mmodler. We'd happily backport it.

Unless @o1y you could reopen this against 5.x?

[o1y]

Done ✅

[mmodler]

You guys rock!