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.

Sign up for GitHub

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

Jump to bottom

ICU-20595 Make icu::TimeZone::AdoptDefault thread safe #649

Merged
merged 1 commit into from
May 13, 2019
Merged

ICU-20595 Make icu::TimeZone::AdoptDefault thread safe #649

merged 1 commit into from
May 13, 2019

Conversation

FrankYFTang
Copy link
Contributor

@FrankYFTang FrankYFTang commented May 9, 2019 •
edited
Loading

Checklist

@FrankYFTang FrankYFTang requested a review from aheninger May 9, 2019 23:36
aheninger
aheninger requested changes May 10, 2019
View reviewed changes
Copy link
Contributor

@aheninger aheninger left a comment

Choose a reason for hiding this comment

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

The comments in timezone.h on set and adoptDefault() need to be updated. The currently document the thread-unsafe behavior that this change fixes.

icu4c/source/i18n/timezone.cpp Outdated Show resolved Hide resolved
icu4c/source/i18n/timezone.cpp Show resolved Hide resolved
@FrankYFTang
Copy link
Contributor Author

PTAL

aheninger
aheninger previously approved these changes May 13, 2019
View reviewed changes
@FrankYFTang
ICU-20595 Make icu::TimeZone::AdoptDefault thread safe
a060620 
correct the mutex

Remove comments about not thread safe
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@FrankYFTang FrankYFTang merged commit cb40d8b into master May 13, 2019
@sffc sffc deleted the ICU-20595 branch May 30, 2019 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jungshik jungshik jungshik left review comments

@aheninger aheninger aheninger approved these changes

Assignees

@aheninger aheninger

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@FrankYFTang @jungshik @aheninger