Re: Cyrus 2.3.16 \Seen flag problem

2011-04-21 Thread Bron Gondwana
On Thu, Apr 21, 2011 at 06:46:25PM +0200, Julien Coloos wrote:
> It may be possible that some other set of conditions would trigger a
> similar issue.
> We tried to think of a clean way to solve it, but provided the
> complexity of that code we gave up (mainly fearing nasty side effetcs
> and introducing more bugs).

Yeah, for sure.

> Maybe that's the reason it was completely rewritten in cyrus 2.4.x
> (which does not have that bug) ? ;)

Correct.

> Unfortunately, moving to 2.4.x version is not yet an option for us :(

Why not?

> So ... is there some cyrus guru expert out there with enough knowledge
> of that part of the code to fix it in the 2.3.x version ?

Yes - I already did once... and I'd rather not touch it with a
bargepole again!  Seriously, there's all sorts of issues with
that bit of the code in 2.3.x, and this is just one of them.

I'd be interested in knowing your reasons for not upgrading to
2.4, because I'd prefer to fix those than ever look at the
2.3 seen management code again!

Bron.


Re: 2.4.8 sync_client crashes

2011-04-21 Thread Bron Gondwana
On Thu, Apr 21, 2011 at 05:30:14PM +0100, Simon Amor wrote:
> On 21 Apr 2011, at 16:51, Rudy Gevaert wrote:
> 
> > As some extra info if I run sync_client -u for that user I get the crash 
> > too.
> > 
> > I have attached a gdb backtrace. I'm not sure if it will be handy.
> > 
> > Rudy
> 
> Hi Rudy,
> 
> After our discussion on IRC, I would suggest the attached patch as a fix - it 
> seems to follow the same syntax as the other places seen_open() is called.
> 
> Disclaimer: My C is very rusty so it might fail horribly. The patch is 
> against clean 2.4.8 sources.
> 
> Perhaps Bron could give it a quick check and if it's correct?

Simon - that's correct.  I could have sworn that made it into 2.4.8 :(
I'll put it on -stable at once.

Bron.



Re: 2.4.8 sync_client crashes

2011-04-21 Thread Rudy Gevaert

On 04/21/2011 06:30 PM, Simon Amor wrote:

On 21 Apr 2011, at 16:51, Rudy Gevaert wrote:


As some extra info if I run sync_client -u for that user I get the crash too.

I have attached a gdb backtrace. I'm not sure if it will be handy.

Rudy


Hi Rudy,

After our discussion on IRC, I would suggest the attached patch as a fix - it 
seems to follow the same syntax as the other places seen_open() is called.

Disclaimer: My C is very rusty so it might fail horribly. The patch is against 
clean 2.4.8 sources.

Perhaps Bron could give it a quick check and if it's correct?

Simon


Hi Simon,

I see this is fixed? on the master branch, commits:

5de9eee60d947243a4b4b2f4eccc63cff2771b30
9dacbfcd6ee077a850570508d0d97f30bfe96640

Bron? :)


Cyrus 2.3.16 \Seen flag problem

2011-04-21 Thread Julien Coloos
Hi,

We encountered a bug in cyrus 2.3.16: mails freshly delivered by LMTP
were already flagged \Seen while not having been read by anyone.
After some investigations, we tracked it down to the code in charge of
updating the seen db: function index_checkseen in imap/index.c file.
That complicated code seems quite prone to issues considering that 5
sets of data are handled:
 - the 'old' seen db, listing UIDs with its initial \Seen state
 - an array in which \Seen state after actions in the current session are stored
 - the 'new' seen db, loaded right before updating its content (to
merge any concurrent sessions changes)
 - the index file data held in static variables of the index.c code section
   -> lets call it index1
 - the mailbox structure, with a possibly newer version of the index file data
   -> lets call it index2

The steps to trigger the issue:
1. The INBOX is opened
2. Mails not already flagged \Seen are read (and so the \Seen flag is
set); alternatively mails are not read but the \Seen flag is set on
them
3. One or more mails are delivered to the mailbox through LMTP
4. The INBOX is closed (with implicit expunge)
Note: using the 'UID' version of the commands (FETCH/STORE) lowers the
probability of encountering the issue because index1 is refreshed for
those commands.

Why those steps trigger the issue ?
Upon closing the INBOX folder, an implicit expunge is performed; a
side effect is that index2 is refreshed. With the other conditions
met:
 - all known mails (not taking into account the ones delivered by
LMTP, which are not visible in index1) now have the \Seen flag
 - the 'old' and 'new' seen db do not differ
the current code updates the seen db believing that UIDs 1 up to
mailbox->last_uid have the \Seen flag. The problem is that mailbox was
refeshed and so all mails delivered by LMTP are now flagged \Seen.

Following the source code with those conditions we would get the
following values:
 - newseen = 0
 - newnext = mailbox->last_uid + 1
 - neweof = 1
 - start = 1
 - inrange = 1
 - usecomma = 0
 - uid = mailbox->last_uid

So we would end in the following part of the code:

if (inrange) {
/* Last message read. */
...
if (!start && uid > 1) start = 1;
if (usecomma++) *save++ = ',';
if (start && start != uid) {
sprintf(save, "%u:", start);
save += strlen(save);
}
sprintf(save, "%u", uid);
save += strlen(save);
...

which does output the range [1,mailbox->last_uid] in the seen db.


It may be possible that some other set of conditions would trigger a
similar issue.
We tried to think of a clean way to solve it, but provided the
complexity of that code we gave up (mainly fearing nasty side effetcs
and introducing more bugs).
Maybe that's the reason it was completely rewritten in cyrus 2.4.x
(which does not have that bug) ? ;)
Unfortunately, moving to 2.4.x version is not yet an option for us :(

So ... is there some cyrus guru expert out there with enough knowledge
of that part of the code to fix it in the 2.3.x version ?


Regards


Re: 2.4.8 sync_client crashes

2011-04-21 Thread Simon Amor
On 21 Apr 2011, at 16:51, Rudy Gevaert wrote:

> As some extra info if I run sync_client -u for that user I get the crash too.
> 
> I have attached a gdb backtrace. I'm not sure if it will be handy.
> 
> Rudy

Hi Rudy,

After our discussion on IRC, I would suggest the attached patch as a fix - it 
seems to follow the same syntax as the other places seen_open() is called.

Disclaimer: My C is very rusty so it might fail horribly. The patch is against 
clean 2.4.8 sources.

Perhaps Bron could give it a quick check and if it's correct?

Simon
-- 
Simon Amor
si...@leaky.org
http://www.leaky.org/


sync_client.patch
Description: Binary data


Re: 2.4.8 sync_client crashes

2011-04-21 Thread Rudy Gevaert
As some extra info if I run sync_client -u for that user I get the crash 
too.


I have attached a gdb backtrace. I'm not sure if it will be handy.

Rudy
(gdb) step
assertionfailed (file=0x451e97 "seen_db.c", line=127, expr=0x451efd "*seendbptr 
== NULL") at assert.c:57
57  assert.c: No such file or directory.
  in assert.c
(gdb) bt
#0  assertionfailed (file=0x451e97 "seen_db.c", line=127, expr=0x451efd 
"*seendbptr == NULL") at assert.c:57
#1  0x0043291a in seen_open (user=0x7fffee71 
"tine^lievr...@ugent.be", flags=, 
seendbptr=0x7fffde48) at seen_db.c:127
#2  0x00407fd2 in do_user_seen (user=0x451e97 "seen_db.c", 
replica_seen=0x7786f0) at sync_client.c:1883
#3  0x0040b1f0 in do_user (userid=0x7fffee71 
"tine^lievr...@ugent.be") at sync_client.c:2020
#4  0x0040d1ab in main (argc=5, argv=) at 
sync_client.c:2927



2.4.8 sync_client crashes

2011-04-21 Thread Rudy Gevaert

Hi,

After upgrading to 2.4.8 the sync_client crashes regularly:

Fatal error: Internal error: assertion failed: seen_db.c: 127: 
*seendbptr == NULL



Please see the attachment for a strace.

I tried reconstructing the mailbox but that didn't help...

On the syncserver I don't see any messages :(


Rudy
open("/mail/mail16/var/imap/lock/domain/u/ugent.be/v/user/victoria^montesrestrepo.lock",
 O_RDWR|O_CREAT|O_TRUNC, 0666) = 8
fcntl(8, F_SETLKW, {type=F_RDLCK, whence=SEEK_SET, start=0, len=0}) = 0
fcntl(3, F_SETLKW, {type=F_RDLCK, whence=SEEK_SET, start=0, len=0}) = 0
fstat(3, {st_mode=S_IFREG|0600, st_size=2315764, ...}) = 0
stat("/mail/mail16/var/imap/mailboxes.db", {st_mode=S_IFREG|0600, 
st_size=2315764, ...}) = 0
fcntl(3, F_SETLKW, {type=F_UNLCK, whence=SEEK_SET, start=0, len=0}) = 0
open("/mail/mail16/imap/domain/u/ugent.be/v/user/victoria^montesrestrepo/cyrus.index",
 O_RDWR) = 9
fstat(9, {st_mode=S_IFREG|0600, st_size=30752, ...}) = 0
mmap(NULL, 40960, PROT_READ, MAP_SHARED, 9, 0) = 0x7f74c7581000
fcntl(9, F_SETLKW, {type=F_RDLCK, whence=SEEK_SET, start=0, len=0}) = 0
stat("/mail/mail16/imap/domain/u/ugent.be/v/user/victoria^montesrestrepo/cyrus.header",
 {st_mode=S_IFREG|0600, st_size=219, ...}) = 0
open("/mail/mail16/imap/domain/u/ugent.be/v/user/victoria^montesrestrepo/cyrus.header",
 O_RDONLY) = 10
fstat(10, {st_mode=S_IFREG|0600, st_size=219, ...}) = 0
mmap(NULL, 219, PROT_READ, MAP_SHARED, 10, 0) = 0x7f74c7694000
munmap(0x7f74c7694000, 219) = 0
fcntl(9, F_SETLKW, {type=F_UNLCK, whence=SEEK_SET, start=0, len=0}) = 0
close(10)   = 0
close(9)= 0
munmap(0x7f74c7581000, 40960)   = 0
close(8)= 0
munmap(0x7f74c750, 528384)  = 0
write(1, "META tine^lievr...@ugent.be\n", 28META tine^lievr...@ugent.be
) = 28
sendto(7, "<182>Apr 21 16:47:33 mail16/sync"..., 74, MSG_NOSIGNAL, NULL, 0) = 74
write(6, "GET META tine^lievr...@ugent.be\r"..., 33) = 33
read(6, "* LSUB (ugent.be!INBOX^Femke \"ug"..., 4096) = 592
write(2, "Fatal error: Internal error: ass"..., 82Fatal error: Internal error: 
assertion failed: seen_db.c: 127: *seendbptr == NULL
) = 82
sendto(7, "<179>Apr 21 16:47:33 mail16/sync"..., 128, MSG_NOSIGNAL, NULL, 0) = 
128
exit_group(75)  = ?
cyrus@cyrprd3:~$  /usr/cyrus-2.4.8/bin/sync_client -C 
/etc/cyrus-ugent/conf/mail16/imapd.conf -l -v -f 
/mail/mail16/var/imap/sync/log-6813 -r -v -v 
MAILBOXES ugent.be!user.victoria^montesrestrepo
META tine^lievr...@ugent.be
Fatal error: Internal error: assertion failed: seen_db.c: 127: *seendbptr == 
NULL
cyrus@cyrprd3:~$ echo $?
75