Re: [boost] [iostreams][io_state_savers] Request for interest in some ios_base basic utilities

Subject: Re: [boost] [iostreams][io_state_savers] Request for interest in some ios_base basic utilities
From: Vicente Botet (vicente.botet_at_[hidden])
Date: 2011-11-10 06:24:18

Artyom Beilis wrote:
> 
> ----- Original Message -----
>> From: Vicente Botet <vicente.botet@>
>> 
>>>  however the second part
>>>  the "state saver" is something very important.
>>> 
>>>  In fact today Boost.DateTime uses for example locale facet to provide
>>> such
>>>  information
>>>  which is wrong by design. If such state saver was around when it
>>> created
>>>  it would be much better.
>>> 
>> 
>> Yes, Boost.Chrono I/O V1 is using this design also. I'm creating a
>> Boost.Chrono v2 design that stores information like duration style,
>> timezone, time format in the ios state.
> 
> On thing - boost.locale already does all this. I mean there is
> manipulators
> to handle time zone, time formats and other things. I'd suggest to take
> a look on them. Because this may be all you need.
> 
> Also it can be extended with some new styles easily if needed like
> for example "duration" style.
> 
> So I'd suggest to use them.
> 

I'll take a look, but I don't think that I will be able to use the
Boost.Locale manipulators, as the intents is to make a proposal for the
standard, so dependence on other libraries should be minimized at least at
the interface level.

>> The inconvenient is the thread safety and inter-module interferences. I
>> think that the locale/facet approach doesn't suffer from these thread and
>> inter-module issues. What do you think?
> 
> No, it has exactly the same problem...
> 

OK.

> The locale facet approach is not correct for other reason.
> 
> Locale/facet object should hold general rules and methods of handling data
> and its facets are shared among multiple instances of std::(i|o)stream
> objects.
> 
> For that reason std::use_facet<Foo> returns Foo const & and not mutable
> reference.
> 
> Now iostream flags should hold (and hold) user specific data like how to
> format
> date/time, number according to rules and they are stream specific not
> locale
> specific.
> 

I agree. This is the reason I'm preparing a chrono I/O v2.

>> 
>> Anyway the IOStream library is not thread safe, so the user can not use
>> them
>> without falling in undefined behavior. 
>> 
> 
> Actually most iostream implementations are thread safe - not in terms
> that same object can be accessed from two threads but rather things
> that are expected to be accessed from different threads like xalloc.
> 
> For example xalloc is thread safe in both MSVC and GCC libraries.
> 

OK, I see. What I don't see is how a library such as Boost.Chrono using
standard libraries that are not ensured to be thread safe, can be thread
safe other than, as Boost.Locale does, force the initialization of all these
data subject to race condition to be initialized before any thread is
launched.

I don't like this kind of design, which forces the user to initialize the
library. I will expect we can do better.
 

>> 
>> 
>>>  So my +1 and big one.
>>> 
>>>  Now some important (extremely important) notes about this code:
>>>  ---------------------------------------------------------------
>>> 
>>> 
>>>  Â Â  Â Â  static inline int index()
>>>  Â Â Â Â Â  {
>>>  Â Â Â Â Â Â Â  static const int v_ = std::ios_base::xalloc();
>>>  Â Â Â Â Â Â Â  return v_;
>>>  Â Â Â Â Â  }
>>> 
>>> 
>>> 
>>>  1.  This code may be not thread safe and may create two different
>>> values
>>>  for two different threads.
>>> 
>>>  Â Â Â  So I'd suggest to provide some global initialization option to at
>>>  least illeminate somehow
>>>  Â Â Â  this problem or to use Boost.Thread's ones initialization.
>>> 
>> 
>> I agree, the code is not thread safe.
>> 
>> 
>> 
>>>  2.  This code can't be used on cross dll boundaries.
>>> 
>>>  Â Â Â  Consider you have function (non-linline) boost::foo::bar in
>>>  libboost_foo.dll that updates manipulator
>>>  Â Â Â  in the exe other inline function boost::foo::baz uses it.
>>> 
>>>  Â Â Â  You'll be surprised what will happen: the "static const int 
>> v_" in the
>>>  index() function
>>>  Â Â Â  would have two instances and would allocate two distinct indexes.
>>> 
>>> 
>> 
>> I know these issues. The final implementation would need a kind of thread
>> safe and multi DLL singleton, maybe based on the holder included in
>> Boost.Flyweight. 
>> 
> 
> I don't know if chrono is header only library or not.
> 
> Just you need to make sure that such library can't be header only.
> As long as it is not header only there will be no such problem.
> 

Well I spent some time to make Boost.Chrono header only. I will be a little
bit disappointed if I can not make a library that uses standard I/O thread
safe other than using some tricks that can not be used in a header only
library.

...snip... 

>>>  That is why Boost.Locale's ios_prop.hpp is hidden in sources.
>>> 
>> 
>> How the fact to hide it makes it more thread or module safe. Could you
>> clarify this?
>> 
> 
> Because, I can initialize all at once and don't live to user an option
> to choose when to do it.
> 
> However once all "static" variables are ready there no more thread 
> issues.
> 
> For example all locale ids and all xalloc ids are initialized once
> globally.
> 
> 

Beside the static data initialization, how do you ensure the access (R or W)
to the ios state flags and locale facets are thread safe? Are you using a
mutex?

Best,
Vicente

--
View this message in context: http://boost.2283326.n4.nabble.com/iostreams-io-state-savers-Request-for-interest-in-some-ios-base-basic-utilities-tp4020844p4023290.html
Sent from the Boost - Dev mailing list archive at Nabble.com.