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

Access to key value pair in tag.Map needs to be thread-safe #105

Closed
krnowak opened this issue Aug 23, 2019 · 4 comments · Fixed by #249
Closed

Access to key value pair in tag.Map needs to be thread-safe #105

krnowak opened this issue Aug 23, 2019 · 4 comments · Fixed by #249
Milestone

Comments

@krnowak
Copy link
Member

krnowak commented Aug 23, 2019

When calling Foreach() on the tag.Map, we gain access to every key and value in the map. We can also use Value() to get access to a value for a specific key. The keys and values returned should essentially be copies of whatever is contained in the map, and usually they are. There are some troublesome members though. Some of them have the pointer-like semantics, so it is possible to mutate them in the Foreach callback or just in the returned object from Value(), and thus mutate the contents of the map itself.

Currently that happens for the keys in the map that contain the BYTES Value. So the map implementation should make a copy of the bytes when giving the access to it.

I'm also wondering if the Type field in registry.Variable should also be copied, because it is an interface, so it also could have some pointer-like semantics when doing shallow copy during Foreach.

@jmacd
Copy link
Contributor

jmacd commented Aug 26, 2019

Perhaps we should remove BYTES, that was suggested elsewhere.

@jmacd
Copy link
Contributor

jmacd commented Aug 26, 2019

The Type variable could probably be eliminated. It's not supposed to contain any state.

@jmacd
Copy link
Contributor

jmacd commented Sep 5, 2019

Oops. This is about allowing mutable value data into the SDK.

@jmacd jmacd reopened this Sep 5, 2019
@jmacd
Copy link
Contributor

jmacd commented Sep 5, 2019

I'd prefer to remove BYTES than to force a copy.

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 a pull request may close this issue.

3 participants