Re: (ITS#5432) syncrepl seg. faults if received a sync cookie with an empty csn= value

Rein Tollevik wrote:
> On Wed, 19 Mar 2008, Howard Chu wrote:
>
>>> syncprov_playlog() in syncprov.c generates such invalid cookies when
>>> replaying a
>>> sessionlog where no entries has been deleted. The patch also fixes this
>>> bug.
>> This should never happen, since the cookie is not generated if there were no
>> deletes recorded.
>
> Well, it do happen anyhow. Using the 2.4.8 release this can easily be
> reproduced with a configuration where the syncrepl provider (with
> sessionlogs enabled) is on a glue backend, the consumer replicates the
> entire glue subtree. Assuming both servers are in sync and stopped,
> start the producer, modify an entry in the subordinate db and then start
> the consumer. The producer will send a cookie with an empty csn= value
> as in this sync debug output:
> slapd starting
> slap_queue_csn: queing 0x418001a0 20080320125116.487494Z#000000#000#000000
> slap_graduate_commit_csn: removing 0x8d6ae0 20080320125116.487494Z#000000#000#000000
> srs csn 20080320124421.642686Z#000000#000#000000
> log csn 20080320125116.487494Z#000000#000#000000
> syncprov_playlog: cookie=rid=007,csn=
> syncprov_search_response: cookie=rid=007,csn=20080320125116.487494Z#000000#000#000000
>
> I have also tested it using the cvs head source, and the good news is
> that this was accidentally fixed (at least partly) by the ITS#5434
> change. I.e it was caused by the be_search() in syncprov_playlog() not
> being propagated to the subordinate database and thereby not finding the
> entry that was modified.
>
> The bad news is that there is still a race condition that would have the
> same effect. If someone deletes the entry that was modified just before
> be_search() is done it will return the same empty cookie. I was able to
> reproduce this in a debugger by setting a break point before be_search(),
> deleting the entry with ldapdelete and in the debugger run next over the
> be_search() call before continuing.
>
> So, I still think it is best to keep delcsn[0].bv_val NULL until an
> actual cookie has been written to it, as slap_compose_sync_cookie() will
> create a cookie with an empty csn= if passed a csn array where bv_len is
> zero but bv_val is non-NULL. Although an alternative and maybe more
> correct fix could be to change slap_compose_sync_cookie() instead?
No. Thanks for your explanation of what you observed. The actual bug is in the
mods processing, when it decides that a modified entry should be treated as a
delete instead. It increments ndel then but doesn't set delcsn in that case.
That's what actually needs to be fixed.
--
-- Howard Chu
Chief Architect, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/