Re: Patch: use a separate lock for observers list in merge folder

[Philip Van Hoof <spam pvanhoof be>]

On Thu, 2007-07-26 at 22:27 +0200, Jose Dapena Paz wrote:
> 	Hi,
> 
> 	This patch maintains a separate lock for observers list in
> TnyMergeFolder. This should prevent locks from inner folders interlock
> with the merge folder itself on notification calls.
> 
> 	Another solution would be modifying the structure and architecture of
> folders to be able to have a tinymail level api lock:
> 
> 	tny_folder_get_mutex ()
> 
> 	We would drop camel folder locks in favor of these ones.
> 
> 	Then we should be able to have a tny_folder_set_mutex method. What
> would we do with this? Make a TnyMergeFolder have the same lock of its
> children and then make access to the merge folder act as it was only a
> folder at lock level.
> 
> 	Would it help a bit with locking problems?

Exposing the locks to the outer API sounds like a terrible idea to me
Jose. Sorry to have to put it like that.

This would make it possible for any application developer to completely
defect Tinymail's inner organs. That's without even mentioning that it's
a bad design to allow this (data encapsulation and that the outer world
of a type should not have to care about the inner details of the type).

I like the patch itself, as it adds granularity to the locking of the
type. The proposal to export the locking infrastructure to the external
API, though, is something I wouldn't accept.

If you have properly tested this patch, and you're sure that the added
granularity does not create new problems, you can commit it.

But do verify this yourself too. I think that indeed it's safe to assume
that the observers can have their own lock, rather than having to rely
on the ugly code locks of the type.

A good rule on locking is the following: lock data, not code. Try to
make locks as fine grained as possible. Although when improving
granularity, make sure you don't miss anything. By leaving open one or
more code paths that can in parallel write to two of the same datas in
memory, you are always creating havoc.

The final idea of tinymail should, indeed, be to have locks that only
lock-out the actual variables themselves. And after that, migrate as
much code as possible to LIFO and FILO queues, locking only the middle
item of the queue. But that's definitely not always easy at all.


-- 
Philip Van Hoof, software developer
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
http://www.pvanhoof.be/blog