List iterator assignment fails, assert iterator not dereferencable

Discussion in 'C++' started by David Bilsby, Oct 8, 2007.

  1. David Bilsby

    David Bilsby Guest

    All

    Apologies for cross posing this but I am not sure if this is a VC 8 STL
    bug or simply an invalid use of the iterator.

    I have a PCI card access class which basically abstracts a third party
    library from the higher level classes. This low level class is used to
    allocate locked pages of memory before doing a DMA. The third party
    library locks memory and returns a pointer to an internal structure,
    which we can simply cast to a (void *).

    My original intention was to have the low level PCI access class store
    these third party pointers away in a list so that if the class is
    destructed and the list is not empty (explicit unlocks have not been
    performed) we can step through the list and tidy up nicely. The list is
    defined in the class as:

    // Some defines to wrap up the list/iterator type definitions.
    typedef void * PCICardDMABufObject;
    typedef list<PCICardDMABufObject> PCICardDMABufHandleList;
    typedef PCICardDMABufHandleList::iterator PCICardDMABufHandle;

    // List type in class.
    PCICardDMABufHandleList *m_pDMAPageList;

    The list is dynamically allocated in the constructor as:

    m_pDMAPageList = new PCICardDMABufHandleList();

    Along with storing the void pointers internally we also want to pass
    these back to the higher level class which wraps up what the specific
    PCI card can do, in this case it has a number of DMA channels. This
    higher level class calls the low level classes lock function and
    receives back an iterator of the void pointer stored in the low level
    classes list as follows:

    // List iterator
    PCICardDMABufHandle hLockedBufHandle;

    // Call to low level PCI class. Iterator returned in last parameter.
    status = m_pPCICardInst->LockDMABuffer(pUserBufStartAddr, bytesToLock,
    dmaDir, &pDMADescPageBufAddr[dmaDescPageBufIndex], &numDMAPages,
    hLockedBufHandle);

    The function prototype in the low level class for this is:

    PCIStatus LockDMABuffer(void *pDataBuf, unsigned int length,
    PCICardDMADir dmaDir,
    PCICardDMAPage *pDMAPages,
    unsigned int *pNumPages,
    PCICardDMABufHandle &hDMABufHandle);

    Later on in the same function we assign the function we assign the list
    iterator to a dynamically allocated array of list iterators for the
    current DMA channel as follows:

    // Store iterator away in higher level class for channel.
    m_phUserDMABufHandles[m_numUserDMABufHandles] = hLockedBufHandle;
    m_numUserDMABufHandles++;

    The array has a fixed max length defined by the largest DMA transfer.
    The array is defined in the higher level class as:

    PCICardDMABufHandle *m_phUserDMABufHandles;

    and is allocated at construction with:

    m_phUserDMABufHandles = new
    PCICardDMABufHandle[PCI_MAX_NUM_USER_DMA_BUFFER_HANDLES];

    The higher level class needs to keep track of which locked pages are
    associated with a give DMA transfer.

    This all works fine up to the point I benchmarked 4 DMA channels from 4
    threads. I carefully protected the low level classes list access, both
    read and write, with a mutex to prevent multiple threads updating the
    list together and from some debugging output to stderr I am confident
    that this is valid.

    The problem I get is when I come to access the iterator stored in the
    high level classes array of iterators once a DMA has completed and I
    need to unlock all the buffers. The low level class is passed the
    iterator back by the high level class, and it tries to dereference the
    iterator to get the void pointer.

    Now this is where the problem takes on an implementation specific
    stance. The code asserts in VC 2005 with an "iterator not
    dereferenceable" at:

    if (this->_Mycont == 0
    || _Ptr == ((_Myt *)this->_Mycont)->_Myhead)
    {
    _DEBUG_ERROR("list iterator not dereferencable");
    _SCL_SECURE_TRAITS_OUT_OF_RANGE;
    }

    _Mycont is 0 when it fails.

    This however is not where the problem actually starts, this is way back
    when the buffer was originally locked and assigned to the array of
    iterators m_phUserDMABufHandles. If I setup a conditional break point in
    VC after the point of assignment at :

    m_phUserDMABufHandles[m_numUserDMABufHandles] = hLockedBufHandle;

    to test _Mycont of m_phUserDMABufHandles then after say 8000 iterations,
    in 4 threads, the breakpoint fires as _Mycont is 0. The _Mycont of the
    hLockedBufHandle however is valid, none 0. If I deliberately move the
    point of execution back to the assignment and try again, the assignment
    works and _Mycont gets a valid value.

    So my problem is why does this not work? And why does it not work after
    working for quite some considerable number of iterations previously?

    Interestingly if I pass the
    m_phUserDMABufHandles[m_numUserDMABufHandles] location directly to the
    LockDMABuffer() call in place of the local hLockedBufHandle it seems to
    work fine!

    Am I doing something bad here with iterators? There seems to be few
    references to what exactly you can and cannot do with an iterator. Most
    references to "not dereferenceable" relate to already erased elements in
    say vectors, this however is a list and from my understanding an
    iterator for an element will remain valid regardless of adds or deletes
    around it in the list. I can also see the element in the list in the
    debugger so I know it is valid, it just seems to be the iterator which
    is broken. It almost looks like a problem with the assignment of the
    iterator.

    Any suggestions, help, sympathy much appreciated.

    David.
     
    David Bilsby, Oct 8, 2007
    #1
    1. Advertising

  2. David Bilsby wrote:
    > The code asserts in VC 2005 with an "iterator not
    > dereferenceable" at:
    >
    > if (this->_Mycont == 0
    > || _Ptr == ((_Myt *)this->_Mycont)->_Myhead)
    > {
    > _DEBUG_ERROR("list iterator not dereferencable");
    > _SCL_SECURE_TRAITS_OUT_OF_RANGE;
    > }
    >
    > _Mycont is 0 when it fails.


    That basically means that either you didn't get this iterator from a
    container or (maybe) that the container ceased to exist. Note that I'm not
    sure with the latter part, it would require that a dying container zeroes
    all the back pointers in its iterators and I'm not sure if the debug
    variant of VC8's stdlib does that. However, that should be trivial to find
    out:

    typedef ... container;

    container::iterator it;
    *it; // should fail, iterator not from container
    {
    container c(1);
    it = c.begin();
    *it; // okay, container still alive
    }
    *it; // should fail, container ceased to exist


    Otherwise, can you distill a minimal example from your code?

    Uli
     
    Ulrich Eckhardt, Oct 8, 2007
    #2
    1. Advertising

  3. David Bilsby

    Andre Kostur Guest

    David Bilsby <> wrote in news:fecvoh$ftr$1$8300dec7
    @news.demon.co.uk:

    > All
    >
    > Apologies for cross posing this but I am not sure if this is a VC 8

    STL
    > bug or simply an invalid use of the iterator.
    >
    > I have a PCI card access class which basically abstracts a third party
    > library from the higher level classes. This low level class is used to
    > allocate locked pages of memory before doing a DMA. The third party
    > library locks memory and returns a pointer to an internal structure,
    > which we can simply cast to a (void *).
    >
    > My original intention was to have the low level PCI access class store
    > these third party pointers away in a list so that if the class is
    > destructed and the list is not empty (explicit unlocks have not been
    > performed) we can step through the list and tidy up nicely. The list

    is
    > defined in the class as:
    >
    > // Some defines to wrap up the list/iterator type definitions.
    > typedef void * PCICardDMABufObject;
    > typedef list<PCICardDMABufObject> PCICardDMABufHandleList;
    > typedef PCICardDMABufHandleList::iterator PCICardDMABufHandle;
    >
    > // List type in class.
    > PCICardDMABufHandleList *m_pDMAPageList;
    >
    > The list is dynamically allocated in the constructor as:
    >
    > m_pDMAPageList = new PCICardDMABufHandleList();


    Why? Why isn't m_pDMAPageList simply a PCICardDMABufHandleList? What
    purpose does dynamically allocating it serve?

    > Along with storing the void pointers internally we also want to pass
    > these back to the higher level class which wraps up what the specific
    > PCI card can do, in this case it has a number of DMA channels. This
    > higher level class calls the low level classes lock function and
    > receives back an iterator of the void pointer stored in the low level
    > classes list as follows:
    >
    > // List iterator
    > PCICardDMABufHandle hLockedBufHandle;
    >
    > // Call to low level PCI class. Iterator returned in last parameter.
    > status = m_pPCICardInst->LockDMABuffer(pUserBufStartAddr, bytesToLock,
    > dmaDir, &pDMADescPageBufAddr[dmaDescPageBufIndex], &numDMAPages,
    > hLockedBufHandle);
    >
    > The function prototype in the low level class for this is:
    >
    > PCIStatus LockDMABuffer(void *pDataBuf, unsigned int length,
    > PCICardDMADir dmaDir,
    > PCICardDMAPage *pDMAPages,
    > unsigned int *pNumPages,
    > PCICardDMABufHandle &hDMABufHandle);
    >
    > Later on in the same function we assign the function we assign the

    list
    > iterator to a dynamically allocated array of list iterators for the
    > current DMA channel as follows:
    >
    > // Store iterator away in higher level class for channel.
    > m_phUserDMABufHandles[m_numUserDMABufHandles] = hLockedBufHandle;
    > m_numUserDMABufHandles++;
    >
    > The array has a fixed max length defined by the largest DMA transfer.
    > The array is defined in the higher level class as:
    >
    > PCICardDMABufHandle *m_phUserDMABufHandles;
    >
    > and is allocated at construction with:
    >
    > m_phUserDMABufHandles = new
    > PCICardDMABufHandle[PCI_MAX_NUM_USER_DMA_BUFFER_HANDLES];


    Again, why? Since PCI_MAX_NUM_USER_DMA_BUFFER_HANDLES appears to be a
    compile-time constant, why bother with the dynamic allocation?

    > The higher level class needs to keep track of which locked pages are
    > associated with a give DMA transfer.
    >
    > This all works fine up to the point I benchmarked 4 DMA channels from

    4
    > threads. I carefully protected the low level classes list access, both
    > read and write, with a mutex to prevent multiple threads updating the
    > list together and from some debugging output to stderr I am confident
    > that this is valid.
    >
    > The problem I get is when I come to access the iterator stored in the
    > high level classes array of iterators once a DMA has completed and I
    > need to unlock all the buffers. The low level class is passed the
    > iterator back by the high level class, and it tries to dereference the
    > iterator to get the void pointer.
    >
    > Now this is where the problem takes on an implementation specific
    > stance. The code asserts in VC 2005 with an "iterator not
    > dereferenceable" at:
    >
    > if (this->_Mycont == 0
    > || _Ptr == ((_Myt *)this->_Mycont)->_Myhead)
    > {
    > _DEBUG_ERROR("list iterator not dereferencable");
    > _SCL_SECURE_TRAITS_OUT_OF_RANGE;
    > }
    >
    > _Mycont is 0 when it fails.
    >
    > This however is not where the problem actually starts, this is way

    back
    > when the buffer was originally locked and assigned to the array of
    > iterators m_phUserDMABufHandles. If I setup a conditional break point

    in
    > VC after the point of assignment at :
    >
    > m_phUserDMABufHandles[m_numUserDMABufHandles] = hLockedBufHandle;
    >
    > to test _Mycont of m_phUserDMABufHandles then after say 8000

    iterations,
    > in 4 threads, the breakpoint fires as _Mycont is 0. The _Mycont of the
    > hLockedBufHandle however is valid, none 0. If I deliberately move the
    > point of execution back to the assignment and try again, the

    assignment
    > works and _Mycont gets a valid value.
    >
    > So my problem is why does this not work? And why does it not work

    after
    > working for quite some considerable number of iterations previously?
    >
    > Interestingly if I pass the
    > m_phUserDMABufHandles[m_numUserDMABufHandles] location directly to the
    > LockDMABuffer() call in place of the local hLockedBufHandle it seems

    to
    > work fine!
    >
    > Am I doing something bad here with iterators? There seems to be few
    > references to what exactly you can and cannot do with an iterator.

    Most
    > references to "not dereferenceable" relate to already erased elements

    in
    > say vectors, this however is a list and from my understanding an
    > iterator for an element will remain valid regardless of adds or

    deletes
    > around it in the list. I can also see the element in the list in the
    > debugger so I know it is valid, it just seems to be the iterator which
    > is broken. It almost looks like a problem with the assignment of the
    > iterator.


    For a list, iterators are invalidated when the element of the list that
    it refers to is erased. I don't see any glaring isses from a Standard
    C++ standpoint. You did mention that you didn't see any problems until
    you went multithreaded. Perhaps your mutexes aren't in the right
    places.
     
    Andre Kostur, Oct 8, 2007
    #3
  4. David Bilsby

    David Bilsby Guest

    Andre Kostur wrote:
    > David Bilsby <> wrote in news:fecvoh$ftr$1$8300dec7
    > @news.demon.co.uk:
    >


    Snip Snip

    >> The list is dynamically allocated in the constructor as:
    >>
    >> m_pDMAPageList = new PCICardDMABufHandleList();

    >
    > Why? Why isn't m_pDMAPageList simply a PCICardDMABufHandleList? What
    > purpose does dynamically allocating it serve?
    >


    Snip Snip

    >> m_phUserDMABufHandles = new
    >> PCICardDMABufHandle[PCI_MAX_NUM_USER_DMA_BUFFER_HANDLES];

    >
    > Again, why? Since PCI_MAX_NUM_USER_DMA_BUFFER_HANDLES appears to be a
    > compile-time constant, why bother with the dynamic allocation?
    >


    Snip Snip

    Points taken, in its current form the fact that we dynamically allocate
    an array of list iterators is not entirely necessary, but not wrong, or
    is it? I think the reason behind this was incase the sizing becomes a
    run-time calculation rather than a compile-time calculation as it is at
    the moment.

    >
    > For a list, iterators are invalidated when the element of the list that
    > it refers to is erased. I don't see any glaring isses from a Standard
    > C++ standpoint. You did mention that you didn't see any problems until
    > you went multithreaded. Perhaps your mutexes aren't in the right
    > places.


    Certainly from the debug code I added the list entry is valid right up
    to the point I try and dereference the bad iterator. The debugger shows
    the entry in the list OK.

    Mutex wise, well I keep thinking this must be the issue, but I think I
    have covered all bases here. The low level class protects the direct
    list access by a mutex and I even added a mutex at the higher level
    class around the call to the low level class function to protect the
    iterator assignment to the array of iterators just incase the assignment
    needs protecting and not just list access or iterator dereferencing.

    David.
     
    David Bilsby, Oct 8, 2007
    #4
  5. David Bilsby schrieb:

    snip

    >> For a list, iterators are invalidated when the element of the list
    >> that it refers to is erased. I don't see any glaring isses from a
    >> Standard C++ standpoint. You did mention that you didn't see any
    >> problems until you went multithreaded. Perhaps your mutexes aren't in
    >> the right places.

    >
    > Certainly from the debug code I added the list entry is valid right up
    > to the point I try and dereference the bad iterator. The debugger shows
    > the entry in the list OK.
    >
    > Mutex wise, well I keep thinking this must be the issue, but I think I
    > have covered all bases here. The low level class protects the direct
    > list access by a mutex and I even added a mutex at the higher level
    > class around the call to the low level class function to protect the
    > iterator assignment to the array of iterators just incase the assignment
    > needs protecting and not just list access or iterator dereferencing.


    Does that mean you lock the high level mutex, call the function that returns the
    iterator, release the mutex, and then start using the returned iterator?
    I yes, then this can bring you into trouble. As soon as you release the mutex,
    another thread could manipulate the list and make your iterator invalid,
    couldn't it?

    Norbert
     
    Norbert Unterberg, Oct 8, 2007
    #5
  6. David Bilsby

    David Bilsby Guest

    Norbert Unterberg wrote:
    >
    > David Bilsby schrieb:
    >
    > snip
    >
    >>> For a list, iterators are invalidated when the element of the list
    >>> that it refers to is erased. I don't see any glaring isses from a
    >>> Standard C++ standpoint. You did mention that you didn't see any
    >>> problems until you went multithreaded. Perhaps your mutexes aren't
    >>> in the right places.

    >>
    >> Certainly from the debug code I added the list entry is valid right up
    >> to the point I try and dereference the bad iterator. The debugger
    >> shows the entry in the list OK.
    >>
    >> Mutex wise, well I keep thinking this must be the issue, but I think I
    >> have covered all bases here. The low level class protects the direct
    >> list access by a mutex and I even added a mutex at the higher level
    >> class around the call to the low level class function to protect the
    >> iterator assignment to the array of iterators just incase the
    >> assignment needs protecting and not just list access or iterator
    >> dereferencing.

    >
    > Does that mean you lock the high level mutex, call the function that
    > returns the iterator, release the mutex, and then start using the
    > returned iterator?
    > I yes, then this can bring you into trouble. As soon as you release the
    > mutex, another thread could manipulate the list and make your iterator
    > invalid, couldn't it?
    >

    No I don't believe it could as an iterator to a list entry is not
    invalidated by any other list additions or deletions unless it is
    carried out on the entry itself. Each thread will only attempt to delete
    locked memory buffers, for which the iterator is the handle, for memory
    they allocated, all memory being separate. There no two threads will
    ever try and delete or access the same list entry, although they may try
    and access the list simultaneously.

    > Norbert


    David.
     
    David Bilsby, Oct 9, 2007
    #6
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Robert Brewer
    Replies:
    1
    Views:
    511
    bsmith
    Nov 7, 2004
  2. Thomas Guettler

    assert 0, "foo" vs. assert(0, "foo")

    Thomas Guettler, Feb 23, 2005, in forum: Python
    Replies:
    3
    Views:
    2,553
    Carl Banks
    Feb 23, 2005
  3. Alex Vinokur

    assert(x) and '#define ASSERT(x) assert(x)'

    Alex Vinokur, Nov 25, 2004, in forum: C Programming
    Replies:
    5
    Views:
    946
    Keith Thompson
    Nov 25, 2004
  4. mkarja

    auto_ptr not dereferencable

    mkarja, Feb 27, 2008, in forum: C++
    Replies:
    13
    Views:
    4,260
    James Kanze
    Feb 28, 2008
  5. ImpalerCore

    To assert or not to assert...

    ImpalerCore, Apr 27, 2010, in forum: C Programming
    Replies:
    79
    Views:
    1,729
    Richard Bos
    May 17, 2010
Loading...

Share This Page