Details
-
Bug
-
Resolution: Unresolved
-
Minor
-
None
-
18.8.0
-
None
-
Unknown
-
Description
Description
When an asynchronous renderer is executed with AsyncRendererConfiguration#setPlaceHolderForced(true) and a finished result is already available in the asynchronous rendering cache, DefaultAsyncRendererExecutor#render returns a placeholder with a new client id, but that client id is never registered in the short-lived cache used to resolve placeholders.
The cached-status branch of render() does:
} else if (asyncAllowed) { // Already running job, associate it with another client return new AsyncRendererExecutorResponse(status, newClientId()); }
The response constructor adds the client id to the status, which works for a job that is still running: the job stores its status for all its clients in AsyncRendererCache when it finishes. For a status that is already finished, nothing stores it again, so getAsyncStatus(id, clientId) finds neither a running job nor an entry for that client, and the async resource answers that there is no status for that id: the placeholder stays empty.
The same happens when the job finishes between the lookup of the running job and the registration of the new client.
Impact
No caller in XWiki Platform reaches this case: the only caller forcing the placeholder (the notifications script service, used by the Notifications macro) uses a renderer which doesn't allow caching. AsyncRendererConfiguration is an internal API. Low priority, but the behaviour is wrong.
Fix notes
- Once the client is added, if the status is finished, store the status in the short-lived cache for that client.
- Don't call AsyncRendererCache#put(status) from that branch: it runs while holding the cache's read lock and put() takes the write lock, which deadlocks with ReentrantReadWriteLock. put() would also register again all the clients that already received the result. A dedicated method storing only the new client in the short-lived cache, without taking the write lock, avoids both problems.
- Add a unit test in DefaultAsyncRendererExecutorTest for a forced placeholder with a finished cached status.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>