Subject: Re: [boost] [thread] Can Boost.Thread use Boost.Atomic without falling on a compatibility issue?
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2013-01-12 12:57:22
Le 12/01/13 18:29, Andrey Semashev a écrit :
> On Saturday 12 January 2013 17:49:38 Vicente J. Botet Escriba wrote:
>> Le 12/01/13 17:09, Andrey Semashev a écrit :
>>> On Saturday 12 January 2013 15:48:21 Vicente J. Botet Escriba wrote:
>>>> Le 12/01/13 14:26, Andrey Semashev a écrit :
>>>>> On Saturday 12 January 2013 12:08:10 Vicente J. Botet Escriba wrote:
>>>>>> Le 12/01/13 11:51, Andrey Semashev a écrit :
>>>>>>> Anyway, can Boost.Thread be modified in such a way so that
>>>>>>> Boost.Atomic
>>>>>>> use is not exposed to the user? E.g. so that call_once invokes a
>>>>>>> compiled
>>>>>>> function implemented within Boost.Thread library that uses
>>>>>>> Boost.Atomic
>>>>>>> to modify the once flag.
>>>>>> yes, this will be great. I don't know Boost.Atomic details to try to do
>>>>>> this. Andrey do you mind to provide a patch that doesn't needs to link
>>>>>> with boost_atomic?
>>>>> I attached the patch (for posix only). It appeared a bit hacky and I'm
>>>>> not
>>>>> sure if you're ok with it.
>>>> Thanks for the quick patch.
>>>> Shouldn't the patch concern only boost/thread/pthread/once.hpp and
>>>> libs/thread/src/pthread/once.hpp?
>>>> Could you send the resulting files also?
>>> There is no libs/thread/src/pthread/once.hpp file as far as I can see.
>> RIght. I meant
>>
>> libs/thread/src/pthread/once.cpp
> It is patched.
>
>>> The
>>> boost/thread/pthread/once.hpp file is no longer needed and can be removed
>>> (as well as boost/thread/win32/once.hpp when win32 version is
>>> implemented). The complete public code is platform-independent and
>>> resides in
>>> boost/thread/once.hpp after the patch is applied.
>> IIRC the problem was on the Posix implementation, so a specific patch
>> for the pthread files will be desirable.
> Ok, I intended to port both Windows and POSIX implementations but we could do
> it just for POSIX variant.
>
> I attached the updated patch to the ticket. The Jamfile will also have to be
> updated to add the dependency on Boost.Atomic. However, the Jamfile in
> Boost.Thread is rather complicated so I didn't do that.
>
>> Anyway, could you send the resulting files to make easier the review?
> The files are attached. These should be boost/thread/posix/once.hpp and
> libs/thread/src/posix/once.cpp, other files are intact.
>
Andrey, the provided patch goes too far for what I was expecting. You
have made a lot of changes, C++11 interface has not been preserved, and
I'm not sure the algorithm is the same. What I was asking for is just a
patch to ensure that
typedef unsigned long uintmax_atomic_t;
is atomic in all platforms.
For the comments about the Jamfile, I deduce that the patch need to link
with Boost.Atomic. I expected a patch that didn't need to link to
Boost.Atomic. I should have missed something.
Could you confirm my very first analysis?
Maybe you could describe your changes?
Vicente