docs(abi): PJ_toolbox_host_vtable_t slots are [stream-thread], not [main-thread] - #162
Merged
Merged
Conversation
…ain-thread]
Comment-only. No ABI, layout, or code change.
Every implemented slot of PJ_toolbox_host_vtable_t was tagged [main-thread],
which is not what the host implements and not how production plugins use it.
Verified in the PJ4 host (pj_datastore/src/plugin_data_host.cpp):
- the write slots (create_data_source, ensure_topic, ensure_field,
append_record, append_bound_record, append_arrow_stream) all bottom out in
WriteCore under lockWriteEngines;
- acquire_catalog_snapshot and read_series_arrow hold engine.lockEngine()
across the whole deep-copy / row-decode;
- register_object_topic and push_owned_object go through ObjectStore, which
is internally thread-safe (store_mutex_ plus a shared_mutex per series).
And toolbox_mosaico drives this vtable from its fetch worker today
(fetch_worker.cpp:454), serialising with its own host_write_mu_ — the
sanctioned shape the tag was denying.
The correct class is [stream-thread], NOT [thread-safe]: each host object
carries one shared error buffer whose pointer is handed back through
PJ_error_t*, so two threads inside slots of the same host object would race on
it. That is exactly the shape PJ_source_write_host_vtable_t already has and is
already tagged with, so this makes the two consistent rather than inventing a
guarantee.
The tag legend is sharpened to carry that meaning: [stream-thread] now spells
out "any ONE thread at a time, typically the plugin's own worker" and the
serialise-if-you-fan-out rule, [main-thread] says GUI ONLY, and [thread-safe]
says concurrently.
Deliberately NOT retagged:
- register_object_topic_on_dataset and set_object_topic_retention — the two
ABI-appended tail slots. NO host implements them (every known vtable leaves
them NULL), so there is no implementation to audit; loosening an unverified
tag is how you sanction a race. The struct comment says so and asks whoever
lands the first implementation to re-tag.
- the colormap registry and settings-store slots — genuinely GUI/QSettings.
- PJ_object_read_host_vtable_t — host wrapper not audited here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
facontidavide
force-pushed
the
docs/write-host-thread-tags
branch
from
August 2, 2026 16:25
64de9d4 to
0ed1199
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Comment-only. No ABI, layout, or code change. A recorded follow-up from the canonical layout-import arc.
The problem
Every implemented slot of
PJ_toolbox_host_vtable_tis tagged[main-thread]. That is not what the host implements, and not how production plugins use it — so the tag actively discourages the shape we ship.What the host actually does
Verified in PJ4's
pj_datastore/src/plugin_data_host.cpp:create_data_source,ensure_topic,ensure_field,append_record,append_bound_record,append_arrow_streamWriteCoreunderlockWriteEnginesacquire_catalog_snapshot,read_series_arrowengine.lockEngine()across the whole deep-copy / row-decoderegister_object_topic,push_owned_objectObjectStore, internally thread-safe (store_mutex_+ ashared_mutexper series)And
toolbox_mosaicodrives this vtable from its fetch worker today (fetch_worker.cpp:454), serialising with its ownhost_write_mu_— precisely the shape the tag was denying.Why
[stream-thread]and not[thread-safe]Each host object carries one shared error buffer, whose pointer is handed back through
PJ_error_t*. Two threads failing concurrently inside slots of the same host object would race on a string the other is reassigning. So the guarantee is "any one thread at a time", not "concurrently".That is exactly the shape
PJ_source_write_host_vtable_talready has — and it is already tagged[stream-thread]. This makes the two consistent rather than inventing a new guarantee.Legend sharpened
The three classes were too terse to carry the distinction, so they now spell it out:
[stream-thread]states the one-thread-at-a-time rule, why off-GUI is sanctioned, and the serialise-if-you-fan-out obligation;[main-thread]says GUI ONLY;[thread-safe]says concurrently.Deliberately NOT retagged
register_object_topic_on_datasetandset_object_topic_retention— the two ABI-appended tail slots. No host implements them; every known vtable leaves themNULL, so there is no implementation to audit. Loosening an unverified tag is how you sanction a race, so they keep the conservative[main-thread], the struct comment says why, and it asks whoever lands the first implementation to re-tag.QSettings-bound.PJ_object_read_host_vtable_t— its host wrapper was not audited here.Testing
Header compile-checked standalone in both modes it is consumed in:
gcc -std=c11 -Wall -Wextraandg++ -std=c++20 -Wall -Wextra, both clean.🤖 Generated with Claude Code