Skip to content
This repository was archived by the owner on Jan 18, 2026. It is now read-only.

fix: panic with concurrent schema parsing#502

Merged
nrwiersma merged 7 commits into
hamba:mainfrom
YousefHagag:fix_panic
Feb 12, 2025
Merged

fix: panic with concurrent schema parsing#502
nrwiersma merged 7 commits into
hamba:mainfrom
YousefHagag:fix_panic

Conversation

@YousefHagag

@YousefHagag YousefHagag commented Feb 10, 2025

Copy link
Copy Markdown
Contributor

Issue: #481

This pull request introduces a new test for concurrent schema parsing, adds a method to merge schema caches, and updates the schema parsing logic to utilize an internal cache. The most important changes are as follows:

Testing Enhancements:

  • concurrent_parse_test.go: Added a new test TestConcurrentParse to validate concurrent schema parsing by running multiple goroutines that parse a test schema. This ensures the thread-safety and correctness of the parsing function.

Schema Cache Enhancements:

  • schema.go: Added a new method AddAll to the SchemaCache struct, which allows merging all schemas from one cache into another. This is useful for combining schema caches without duplicating entries.

Schema Parsing Logic Enhancements:

  • schema_parse.go: Updated the ParseBytesWithCache function to use an internal SchemaCache for intermediate parsing steps. This change helps in isolating the parsing process and then merging the results back into the original cache, improving the modularity and maintainability of the code.

@YousefHagag YousefHagag changed the title Fix panic with concurrent schema parsing fix: panic with concurrent schema parsing Feb 10, 2025
Comment thread schema_parse.go Outdated
Comment thread schema_parse.go Outdated
Comment thread concurrent_parse_test.go Outdated
"github.com/stretchr/testify/require"
)

func TestConcurrentParse(t *testing.T) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add this to schema_test.go. The naming separates it from what it is trying to test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have moved the test, also I have moved the schema to a file, it still reproduces the issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants