Bug 47647

I just checked my Nokia N9 and this bug exists on the device too. I have /lots/ of avatars in ~/.cache/telepathy/avatars/gabble/jabber with the same name and different suffix, exactly the same files. It's a normal device, no modifications done to it.

Telepathy-gabble is 0.12.6 and telepathy-qt4 is 0.6.5 on the device.

I think this have nothing to do with apparmor.. since I'm using chakra (so no apparmor here) and also experience same problem here.

Created attachment 66794 [details] [review]
Remove old avatar file before rename

From qt docs:
If a file with the name newName already exists, rename() returns false (i.e., QFile will not overwrite it).

Created attachment 66805 [details] [review]
update the patch.

Sorry for a little typo without "!"

(In reply to comment #3)
> From qt docs:
> If a file with the name newName already exists, rename() returns false
> (i.e., QFile will not overwrite it).

Indeed, this is the bug. For some reason, avatars are retrieved multiple times, even from the same application, and only the first time succeeds. Later on, temporary files are written but not renamed to the proper avatar file name and not removed either.

This patch addresses the problem, but it is not correct in the sense that it is not atomic. There could be a race condition between two processes trying to retrieve the same avatar at the same time.

This trick with QTemporaryFile is based on g_file_set_contents() that tp-glib uses, which guarantees an atomic operation, but only on unix. On windows, it behaves like this patch (it removes the original file first, then renames). Now, we could implement a similar function in tp-qt, using the posix rename() function, but it wouldn't work on windows for the same reason that it doesn't work for g_file_set_contents() on windows.

Therefore, a better solution, imho, would be to just ignore the new avatar data if the file already exists on the hard drive, since it is guaranteed that two avatars with the same token and from the same CM/protocol have exactly the same contents. I don't really see a reason why we should overwrite the existing data with the same data. Opinions?

Created attachment 67631 [details] [review]
New proposed patch

Well, the problem is that people may change theirs avatar, so we still need to update it.

Even if two process will access the same avatar at the same time, I think it won't harm much right?

(In reply to comment #7)
> Well, the problem is that people may change theirs avatar, so we still need
> to update it.

The idea is that if the avatar token stays the same, it's the same avatar; if I change my avatar, the token changes.

A typical token would be the MD5 or SHA1 of the image, or some encoding of the tuple (contact ID, timestamp of last change).

(In reply to comment #8)
> (In reply to comment #7)
> > Well, the problem is that people may change theirs avatar, so we still need
> > to update it.
> 
> The idea is that if the avatar token stays the same, it's the same avatar;
> if I change my avatar, the token changes.
> 
> A typical token would be the MD5 or SHA1 of the image, or some encoding of
> the tuple (contact ID, timestamp of last change).

Ah.. I'm not familar with telepathy, sorry for my dumb question.
But will the out of date avatar be ever cleaned up?

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > Well, the problem is that people may change theirs avatar, so we still need
> > > to update it.
> > 
> > The idea is that if the avatar token stays the same, it's the same avatar;
> > if I change my avatar, the token changes.
> > 
> > A typical token would be the MD5 or SHA1 of the image, or some encoding of
> > the tuple (contact ID, timestamp of last change).
> 
> Ah.. I'm not familar with telepathy, sorry for my dumb question.
> But will the out of date avatar be ever cleaned up?

No, and that's another problem. Originally I thought that this bug report had to do with the fact that old avatars are never removed, but it turns out there is also this QTemporaryFile rename issue.

So.. any updates on this?

This can be serious problem since it slowly fill up users hard disk (I have 2 gigabyte there after last check) and hope this can be resolved soon.