Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Invalidate iterator if modifying this #355

Closed
wants to merge 1 commit into from

2 participants

@glance-

If subtract is called on itself, prefix and this is the same and the
iterator is invalid after modifying the underlaying object.

@keithw
Owner

Very nice catch.

(1) How do you test and reproduce it? What platform? We have not seen this with libstdc++, libc++, and whatever Solaris uses.

(2) Can we just detect the case of prefix == this, and call actions.clear() and return at the beginning of the function? Maybe we would rather do it that way.

(3) Is there any other place in the codebase where this category of bug may be lurking?

(4) I'm glad those asserts finally proved good for something!

@glance- glance- We can't iterate over the object we are modifying
If subtract is called on itself, prefix and this is the same and the
iterator is invalid after modifying the underlaying object.

Instead just clear everyting.
1fad5a4
@glance-
  1. It showed up when mosh was compiled with xlC on AIX5 tested with both 11.1 and 12.1, using libC.a (xlC c++ runtime)

  2. Updated patch to your suggestion. Works just fine.

  3. As shown in the updated patch, the way to steer clear from that kind of bug is probably to add a assertion that the deque we are iterating over and the deque we are modifying isn't the same.

@keithw keithw closed this pull request from a commit
@glance- glance- We can't iterate over the object we are modifying
If subtract is called on itself, prefix and this is the same and the
iterator is invalid after modifying the underlaying object.

Instead just clear everyting.

Closes #354. Closes #355.
70a7c80
@keithw keithw closed this in 70a7c80
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 30, 2012
  1. @glance-

    We can't iterate over the object we are modifying

    glance- authored
    If subtract is called on itself, prefix and this is the same and the
    iterator is invalid after modifying the underlaying object.
    
    Instead just clear everyting.
This page is out of date. Refresh to see the latest.
Showing with 6 additions and 0 deletions.
  1. +6 −0 src/statesync/user.cc
View
6 src/statesync/user.cc
@@ -43,9 +43,15 @@ using namespace ClientBuffers;
void UserStream::subtract( const UserStream *prefix )
{
+ // if we are subtracting ourself from ourself, just clear the deque
+ if ( this == prefix ) {
+ actions.clear();
+ return;
+ }
for ( deque<UserEvent>::const_iterator i = prefix->actions.begin();
i != prefix->actions.end();
i++ ) {
+ assert( this != prefix );
assert( !actions.empty() );
assert( *i == actions.front() );
actions.pop_front();
Something went wrong with that request. Please try again.