Conversation
- SpecsIo.createTempDirectory(prefix): unique directory allocated by the OS under the user temp folder, registered for deletion on exit as a backstop, so callers no longer hand-roll UUID/timestamp names - SpecsIo.getOsCacheFolder(): user-level cache folder following OS conventions (XDG_CACHE_HOME, ~/Library/Caches, %LOCALAPPDATA%) - SpecsSystem.isMac()
Add a backed store that decodes individual fields on first access, preserves deferred copies, and lets typed writes replace pending values. Verify storage semantics and run the full jOptions suite and coverage gate.
Build definition key maps before publication and use a concurrent cache for slot indexes. Parallel first readers previously observed incomplete maps, causing lazy AST imports to reject valid fields. Add a concurrent first-use test that reproduces the old failure.
Remove the FlatBuffers lazy datastore and slot resolution hooks from the eager production branch. Preserve the concurrency regression using ordinary stores; the complete lazy implementation remains on lazy-flatbuffers-experiment. Validation: jOptions tests and JaCoCo coverage gate pass.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff2b543328
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.appName = appName; | ||
| this.sections = new ArrayList<>(sections); | ||
| this.defaultData = defaultData; | ||
| this.keyMap = new HashMap<>(StoreDefinition.super.getKeyMap()); |
There was a problem hiding this comment.
StoreDefinition.super.getKeyMap() calls getKeys() virtually. If a downstream AStoreDefinition subclass overrides getKeys() and reads fields initialized after super(...), construction now sees uninitialized state and can throw or permanently cache an incomplete map; previously the lazy lookup occurred only after construction. Populate the map directly from the initialized sections or through a non-overridable base method.
Useful? React with 👍 / 👎.
| File tempFolder = Files.createTempDirectory(getTempFolder().toPath(), prefix).toFile(); | ||
|
|
||
| // Backstop deletion, in case the owner does not get the chance to delete it | ||
| deleteOnExit(tempFolder); |
There was a problem hiding this comment.
When callers put files into the returned directory, this registration does not provide the documented shutdown backstop: deleteOnExit traverses the directory immediately while it is still empty, so subsequently created children are never registered and deletion of the now-nonempty directory fails at JVM exit. Use cleanup that traverses at shutdown, or otherwise ensure later contents are removed before deleting the directory.
Useful? React with 👍 / 👎.
1 participant
Concurrent eager AST construction can race while shared datastore key metadata is initialized. Publish the key map during construction and use
ConcurrentHashMap.computeIfAbsentfor shared key indexes. The production branch uses ordinary eager stores; deferred store experiments are preserved onlazy-flatbuffers-experiment.Includes the temporary/cache folder helpers used by the consumer resource lifecycle. Stacked on #34.
Validated at
ff2b543328c585376b64aded0d15778b6c67d59b: 1,737 tests passed and JaCoCo verification passed, including the concurrent initialization regression.