Page MenuHomePhabricator

mitk::LocaleSwitch promotes illpossed pattern and excessive use reduces thread savety
Open, LowPublic

Description

  1. mitk::LocaleSwitch is not thread safe. This should be stated explicitly in its documentation. Reason: it uses std::setlocale which sets the global locale state. If LocaleSwitch is used in different threads in parallel we have a race condition. :(
  2. mitkLocaleSwitch promotes in its documentation an illposed usage:
std::string toString(int number)
{
  mitk::LocaleSwitch localeSwitch("C");// installs C locale until the end of the function

  std::stringstream parser;
  parser << number;

  return parser.str();
}

But we have a better and thread-safe option for string to number conversion or vice versa.

std::string toString(int number)
{
  std::ostringstream parser;
  parser.imbue(std::locale("C"))
  parser << number;
  return parser.str();
}

mitk::LocaleSwitch has its rational. Especial in IO (its origin) where we have to deal with third party code where we have no other option to adjust the locale. But over the time LocaleSwitch has "missused" in code parts (especially for stringstream operations), where we have a better pure-local option.

I would propose to reduce the use of LocaleSwitch to situation where it is realy needed. Therfore we should:

  • alter the documentation of LocaleSwitch to sensiblize for the problem and other thread safe options.
  • Check code for situations where LocaleSwitch is used for just locale stringstream based conversions and use imbue instead.

Event Timeline

Helper like done for DICOM values (where we know the fixed locale could also be an option). See T24296: Add helper to make the conversion of DICOMProperty values into numbers.

kislinsk added a project: Restricted Project.

I have altered the documentation already while doing T27317.

kislinsk added a project: Auto-closed.
kislinsk added a subscriber: kislinsk.

Hi there! ๐Ÿ™‚

This task was auto-closed according to our Task Lifecycle Management.
Please follow this link for more information and don't forget that you are encouraged to reasonable re-open tasks to revive them. ๐Ÿš‘

Best wishes,
The MITK devs

floca removed a project: Auto-closed.