Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix thread-safety bug in Obj.getAllKeys #217

Merged

Conversation

JoshRosen
Copy link
Contributor

This PR fixes a thread-safety bug in Val.obj.getAllKeys, similar to the previous fix in #136. It looks like this particular site was overlooked in that earlier PR.

I spotted this potential issue while reading through the code, not via actual use.

@stephenamar-db stephenamar-db merged commit 9f56e1f into databricks:master Nov 26, 2024
1 check passed
@JoshRosen JoshRosen deleted the obj-getalllkeys-thread-safety branch December 4, 2024 00:14
@JoshRosen
Copy link
Contributor Author

On reflection, I'm actually unsure about whether this race condition can occur in practice:

If the parse cache is the means by which objects can be shared across threads and that can only cache static objects, then:

  • Static objects have allKeys pre-assigned,
  • thus we'll never attempt to initialize allKeys,
  • thus we'll not hit a race condition here.

In contrast, I think that getValue0 could potentially be invoked for a cached static object if addSuper is called, which could happen if we're using the binary + operation for object-object concatenation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants