On Sun, September 24, 2006 22:19, Bart Samwel wrote:
> It looks like connection_base::RemoveTrigger() is failing. I guess we
> have R.second = R.first = m_Triggers.end(), in which case this line:
>
> const TriggerList::iterator i = find(R.first, R.second, E);
>
> may fail in some implementations, but definitely this one:
>
> if (m_Conn && (R.second == ++R.first))
> Exec(("UNLISTEN \"" + T->name() + "\"").c_str(), 0);
>
> Jeroen? Any idea if this can ever happen?
Good work... I'm not sure though: why should that find() fail if R.first
and R.second are both equal to m_Triggers.end()? AFAICS find(x, x, y) is
a find() on an empty range, and so should always return x without it ever
being dereferenced or incremented. Shouldn't behaviour be perfectly
well-defined even if x is an end()?
Now *assuming* that that is true, then the "++R.first" would never run
because i, the value returned by find(), would be equal to R.second, i.e.
the end of the search range.
Nevertheless, I think you may be on to something: let's look at a case
where R.first and R.second are _not_ both equal to m_Triggers.end(), and
where we do get to that "++R.first" statement. What if the find()
returned R.first? It's quite likely actually, since that's what happens
if there's only one trigger of the same name.
In that case, i == R.first, so we'd be doing a "m_Triggers.erase(R.first)"
followed by "++R.first". That can't be good.
Ah, I think I have it. See that comment about the correction by Dragan
Milenkovic? He found a race condition that he suggested I fix by
switching those two statements around. But that breaks R.first if it
points to the same element as i.
Luckily it's easy to fix: the "++R.first" was just a way of getting around
the fact that "R+1" wasn't supported for this iterator type.
So Zhao (or Xiaofeng, which should I use?), could you test this fix for
src/connection_base.cxx, lines 591-597?
{
// Erase first; otherwise a notification for the same trigger may yet come
// in and wreak havoc. Thanks Dragan Milenkovic.
+ const bool gone = (m_Conn && (R.second == ++R.first));
m_Triggers.erase(i);
- if (m_Conn && (R.second == ++R.first))
+ if (gone)
Exec(("UNLISTEN \"" + T->name() + "\"").c_str(), 0);
}
All that that does is move the computation of the "if" condition up before
the erase(). If our analysis is correct, that should fix the problem.
Jeroen
_______________________________________________
Libpqxx-general mailing list
[email protected]
http://gborg.postgresql.org/mailman/listinfo/libpqxx-general
