[Bug 104956] dimap: sudden mail loss

[Pierre Habouzit]

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
         
http://bugs.kde.org/show_bug.cgi?id=104956         




------- Additional Comments From madcoder debian org  2006-06-28 23:12 -------
Le mer 28 juin 2006 22:29, Pierre Habouzit a écrit :

> I'm almost sure that fixing that will fix 99% of the dimap mail
> losses. I'll see if I can create a patch of this, but I'm not very
> sure, I don't know kmail's code at all.


to add the "Coup de Grâce":

int KMFolderCachedImap::writeUidCache()
{
  if( uidValidity().isEmpty() || uidValidity() == "INVALID" ) {
    // No info from the server yet, remove the file.
    if( QFile::exists( uidCacheLocation() ) )
      unlink( QFile::encodeName( uidCacheLocation() ) );
    return 0;
  }

  QFile uidcache( uidCacheLocation() );
  if( uidcache.open( IO_WriteOnly ) ) {
    QTextStream str( &uidcache );
    str << "# KMail-UidCache V" << UIDCACHE_VERSION << endl;
    str << uidValidity() << endl;
    str << lastUid() << endl;
    uidcache.flush();
    fsync( uidcache.handle() ); /* this is probably overkill */
    uidcache.close();
    return 0;
  } else {
    return errno; /* does QFile set errno? */
  }
}

is completely broken: it uses QTextStream that has absolutely *NO* error 
handling, so you may open a file, not beeing able to write to it due to 
NFS errors, and close it without problems, and return with a nice 0 
status (that is ignored anyway). yay \o/


uidcache in kmail is FUBAR from head to toe, and IS the culprit without 
any doubt possible of all mail losses in dimap. The fact that it's hard 
to reproduce is that it causes problems iff kmail thought it had 
written the uidcache, and it didn't. And kmail has two ways to write an 
uid cache:

 * on timers (I've seen that in the source) ==> hence the fact that a
   lot of reports say that very big directories trigger the bug more
   easily).
 * or when the sync is done completely.

In fact, this has nothing to do with the fact that kmail uses implicit 
mail deletion (or at least, it's not the worse thing in all the bad 
designs that lead to that nasty bug).



at least for writeUidCache, I'd suggest something like:

int KMFolderCachedImap::writeUidCache()
{
  if( uidValidity().isEmpty() || uidValidity() == "INVALID" ) {
    // No info from the server yet, remove the file.
    if (QFile::exists(uidCacheLocation())) {
      /* YES UNLINK CAN FAIL TOO AND IT'S A PROBLEM TOO !!! */
      return unlink( QFile::encodeName( uidCacheLocation() ) );
    }
  }

  /* yes C APIs are good when we need to deal with errors... */
  FILE *f = fopen(uidCacheLocation(), "w");
  if (!f)
    return -1;

  if (fprintf(f, "# Kmail-UidCache V%d\n", UIDCACHE_VERSION) < 0
  ||  fprintf(f, "%ld\n", uidValidity()) < 0
  ||  fprintf(f, "%ld\n", lastUid()) < 0)
  ||  fflush(f))
  {
      fclose(f);
      return -1;
  }

  /* YES FCLOSE CAN FAIL IF THERE IS QUOTAs INVOLVED !!! */
  return fclose(f);
}


and *EVERY* single call to writeUidCache MUST read the returned value, 
and act correctly if it's non zero.

I'll now let the kmail coders deal with what "act correctly" means, the 
code is not localized enough so that I can provide a useful patch.