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 2 issues related to thread safety in CustomProperties #840

Merged
merged 1 commit into from
Oct 11, 2018

Conversation

guperrot
Copy link
Member

@guperrot guperrot commented Oct 11, 2018

  • Avoid concurrent modification exceptions while and after calling setCustomProperties by taking a snapshot of properties when we call setCustomProperties.
  • Avoid bypassing the size limit by inserting keys from multiple threads.

@guperrot guperrot force-pushed the fix/custom-properties-threadsafe branch from cb45279 to a4ab91a Compare October 11, 2018 01:37
@microsoft microsoft deleted a comment from coveralls Oct 11, 2018
@guperrot
Copy link
Member Author

guperrot commented Oct 11, 2018

It's not the same fix as in EventProperties as we don't do traversals in multiple places for CustomProperties (for EventProperties we traverse also when using inheritance in targets and that would have been harder to use synchronized for that) as opposed to a single copy traversal for snapshot.

Plus we have the size check that is done at set time and needs to be atomic as well which was not achieved by using concurrent map (in event properties, the size check is done later on a snapshot copy and only for AppCenter).

Also concurrent map was requiring to use an object marker for null instead of null so it was introducing more changes.

FYI the alternate version of the fix was cb45279.

@guperrot guperrot force-pushed the fix/custom-properties-threadsafe branch 2 times, most recently from 45964a9 to 928668d Compare October 11, 2018 01:45
@guperrot guperrot force-pushed the fix/custom-properties-threadsafe branch from 928668d to 0440f4f Compare October 11, 2018 01:51
@jaeklim jaeklim merged commit d3bb45b into develop Oct 11, 2018
@jaeklim jaeklim deleted the fix/custom-properties-threadsafe branch October 11, 2018 16:31
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.

None yet

2 participants