Skip to content

Fix thread-safety bug in Obj.getAllKeys#217

Merged
stephenamar-db merged 1 commit intodatabricks:masterfrom
JoshRosen:obj-getalllkeys-thread-safety
Nov 26, 2024
Merged

Fix thread-safety bug in Obj.getAllKeys#217
stephenamar-db merged 1 commit intodatabricks:masterfrom
JoshRosen:obj-getalllkeys-thread-safety

Conversation

@JoshRosen
Copy link
Copy Markdown
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
@JoshRosen JoshRosen deleted the obj-getalllkeys-thread-safety branch December 4, 2024 00:14
@JoshRosen
Copy link
Copy Markdown
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

2 participants