[boost] Re: [shared_ptr] possible bug in sp_counted_base.hpp

From: Rene Rivera (grafik.list_at_[hidden])
Date: 2005-04-14 13:36:20

Peter Dimov wrote:
> Rene Rivera wrote:
> 
>>>    asm
>>>    {
>>>        mov esi, [pw]
>>>        lock
>>>        inc [esi]
>>>    }
> 
> Everything looks correct to me, except I'd put the lock prefix before 
> the instruction. GCC needs it on its own line but the x86 way is 'lock 
> inc [esi]'.
> 
> Unless you can post a disassembly, I'm out of ideas. :-)

I looked at the disassembly of some of the tests and found one problem. 
CW was using the incorrect access size in some cases, BYTE instead of 
DWORD. So if I make that explicit everything works.

I running the timing tests again, with some changes to see if I can get 
more representative numbers. But attached is the end result.

-- 
-- Grafik - Don't Assume Anything
-- Redshift Software, Inc. - http://redshift-software.com
-- rrivera/acm.org - grafik/redshift-software.com
-- 102708583/icq - grafikrobot/aim - Grafik/jabber.org

#ifndef BOOST_DETAIL_SP_COUNTED_BASE_CW_X86_HPP_INCLUDED
#define BOOST_DETAIL_SP_COUNTED_BASE_CW_X86_HPP_INCLUDED

// MS compatible compilers support #pragma once

#if defined(_MSC_VER) && (_MSC_VER >= 1020)
# pragma once
#endif

//
//  detail/sp_counted_base_gcc_x86.hpp - g++ on 486+ or AMD64
//
//  Copyright (c) 2001, 2002, 2003 Peter Dimov and Multi Media Ltd.
//  Copyright 2004-2005 Peter Dimov
//
//  Distributed under the Boost Software License, Version 1.0. (See
//  accompanying file LICENSE_1_0.txt or copy at
//  http://www.boost.org/LICENSE_1_0.txt)
//
//
//  Lock-free algorithm by Alexander Terekhov
//
//  Thanks to Ben Hitchings for the #weak + (#shared != 0)
//  formulation
//

#include <typeinfo>

namespace boost
{

namespace detail
{

inline int atomic_exchange_and_add( int * pw, int dv )
{
    // int r = *pw;
    // *pw += dv;
    // return r;

    asm
    {
        mov esi, [pw]
        mov eax, dv
        lock xadd dword ptr [esi], eax
    }
}

inline void atomic_increment( int * pw )
{
    //atomic_exchange_and_add( pw, 1 );

    asm
    {
        mov esi, [pw]
        lock inc dword ptr [esi]
    }
}

inline int atomic_conditional_increment( int * pw )
{
    // int rv = *pw;
    // if( rv != 0 ) ++*pw;
    // return rv;

    asm
    {
        mov esi, [pw]
        mov eax, dword ptr [esi]
    L0:
        test eax, eax
        je L1
        mov ebx, eax
        inc ebx
        lock
        cmpxchg dword ptr [esi], ebx
        jne L0
    L1:
    }
}

class sp_counted_base
{
private:

    sp_counted_base( sp_counted_base const & );
    sp_counted_base & operator= ( sp_counted_base const & );

    int use_count_;        // #shared
    int weak_count_;       // #weak + (#shared != 0)

public:

    sp_counted_base(): use_count_( 1 ), weak_count_( 1 )
    {
    }

    virtual ~sp_counted_base() // nothrow
    {
    }

    // dispose() is called when use_count_ drops to zero, to release
    // the resources managed by *this.

    virtual void dispose() = 0; // nothrow

    // destroy() is called when weak_count_ drops to zero.

    virtual void destroy() // nothrow
    {
        delete this;
    }

    virtual void * get_deleter( std::type_info const & ti ) = 0;

    void add_ref_copy()
    {
        atomic_increment( &use_count_ );
    }

    bool add_ref_lock() // true on success
    {
        return atomic_conditional_increment( &use_count_ ) != 0;
    }

    void release() // nothrow
    {
        if( atomic_exchange_and_add( &use_count_, -1 ) == 1 )
        {
            dispose();
            weak_release();
        }
    }

    void weak_add_ref() // nothrow
    {
        atomic_increment( &weak_count_ );
    }

    void weak_release() // nothrow
    {
        if( atomic_exchange_and_add( &weak_count_, -1 ) == 1 )
        {
            destroy();
        }
    }

    long use_count() const // nothrow
    {
        return static_cast<int const volatile &>( use_count_ );
    }
};

} // namespace detail

} // namespace boost

#endif  // #ifndef BOOST_DETAIL_SP_COUNTED_BASE_GCC_X86_HPP_INCLUDED