#36 Possible invalid iterator in XmlRpcDispatch::work()

[Christian]

There is a bug in the
XmlRpcDispatch::work(double timeout)
method.
For example, if the communication breaks down the
src->handleEvent
calls
XmlRpcClient::setupConnection()

This invalidates the iterator in the Process events loop => software crashes.
I fixed this by introducing an additional test after the event handling, see below:

// Process events
for (it=_sources.begin(); it != _sources.end(); )
{
  SourceList::iterator thisIt = it++;
  XmlRpcSource* src = thisIt->getSource();
  int fd = src->getfd();
  unsigned newMask = (unsigned) -1;
  if (fd <= maxFd) {
    // If you select on multiple event types this could be ambiguous
    if (FD_ISSET(fd, &inFd))
      newMask &= src->handleEvent(ReadableEvent);
    if (FD_ISSET(fd, &outFd))
      newMask &= src->handleEvent(WritableEvent);
    if (FD_ISSET(fd, &excFd))
      newMask &= src->handleEvent(Exception);

    // handleEvent may call setupConnection => check iterator
    if ( thisIt._Mycont==NULL )
      continue;

    if ( ! newMask) {
      _sources.erase(thisIt);  // Stop monitoring this one
      if ( ! src->getKeepOpen())
        src->close();
    } else if (newMask != (unsigned) -1) {
      thisIt->getMask() = newMask;
    }
  }
}

[Yan Meunier]

I know this is an old project, and low activity, but this library is pretty easy to use, in case others run into this. The above fix didn't seem to work for me while running in Windows (VC++ 2017) while in Release. Works fine in Debug. Looks like Mycont always return nullptr in release. Rather then trying to check the iterator a copy of the sources should be used to iterator through. When the source no longer needs to be monitored it can be removed from the copied list.

In the case that the handleEvent causes the setupConnection to be called, the new source will be added to original sources list and.

auto src_cpy = _sources;
// Process events
for (it= src_cpy.begin(); it != src_cpy.end(); )
{
    SourceList::iterator thisIt = it++;
    XmlRpcSource* src = thisIt->getSource();
    int fd = src->getfd();
    unsigned newMask = (unsigned) -1;
    if (fd <= maxFd) {
    // If you select on multiple event types this could be ambiguous
    if (FD_ISSET(fd, &inFd))
        newMask &= src->handleEvent(ReadableEvent);
    if (FD_ISSET(fd, &outFd))
        newMask &= src->handleEvent(WritableEvent);
     if (FD_ISSET(fd, &excFd))
         newMask &= src->handleEvent(Exception);

      if ( ! newMask) {
          removeSource(src); // remove it from the master list

        if ( ! src->getKeepOpen())
            src->close();
      } else if (newMask != (unsigned) -1) {

      thisIt->getMask() = newMask;
      }
    }
 }