List iterator assignment fails, assert iterator not dereferencable

[David Bilsby]

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.

 

[Ulrich Eckhardt]

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

 

[Andre Kostur]

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.

 

[David Bilsby]

@news.demon.co.uk:

Snip Snip

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.

 

[Norbert Unterberg]

David Bilsby schrieb:

snip

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

 

[David Bilsby]

David Bilsby schrieb:

snip


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.