Skip to content

ICU-21591 Release lock in SimpleDateFormat::tzFormat in case of failure#1701

Merged
aheninger merged 1 commit intounicode-org:mainfrom
aheninger:SimpleDateFormat_locking
Apr 28, 2021
Merged

ICU-21591 Release lock in SimpleDateFormat::tzFormat in case of failure#1701
aheninger merged 1 commit intounicode-org:mainfrom
aheninger:SimpleDateFormat_locking

Conversation

@aheninger
Copy link
Contributor

@aheninger aheninger commented Apr 25, 2021

Also remove the use of the unsafe double-checked lock idiom in the same
function, SimpleDateFormat::tzFormat(). Synchronization now always uses a
mutex, which is slower, but in the context of format or parse operations,
shouldn't be significant.

Added synchronization to one more unsafe direct reference to a const
SimpleDateFormat::fTimeZoneFormat. In the assignment operator.

Checklist
  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-21591
  • Required: The PR title must be prefixed with a JIRA Issue number.
  • Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • Required: Each commit message must be prefixed with a JIRA Issue number.
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

@aheninger
Copy link
Contributor Author

Followup replacement for #1699. Hoping to avoid making extra work for filipnavara, who identified the original problem.

Copy link
Member

@markusicu markusicu left a comment

Choose a reason for hiding this comment

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

Thanks!
lgtm pse squash

Also remove the use of the unsafe double-checked lock idiom in the same
function, SimpleDateFormat::tzFormat(). Synchronization now always uses a
mutex, which is slower, but in the context of format or parse operations,
shouldn't be significant.

Added synchronization to one more unsafe direct reference to a const
SimpleDateFormat::fTimeZoneFormat. In the assignment operator.
@aheninger aheninger force-pushed the SimpleDateFormat_locking branch from a71260c to 137b314 Compare April 27, 2021 19:58
@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

Copy link
Member

@markusicu markusicu left a comment

Choose a reason for hiding this comment

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

lgtm tnx

@aheninger aheninger merged commit 7577899 into unicode-org:main Apr 28, 2021
@aheninger aheninger deleted the SimpleDateFormat_locking branch April 28, 2021 01:12
daniel-ju added a commit to microsoft/icu that referenced this pull request May 3, 2021
Cherry-pick: ICU-21591 Release lock in SimpleDateFormat::tzFormat in case of failure

This cherry-picks the upstream fix to resolve the deadlock in SimpleDateFormat::tzFormat.

Upstream ticket: https://unicode-org.atlassian.net/browse/ICU-21591
Upstream PR: unicode-org/icu#1701
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