[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Locking and errors (and deserialization) in inprocess-cache

From: Daniel Shahaf <danielsh_at_elego.de>
Date: Thu, 19 May 2011 17:01:15 +0200

In inprocess-cache.c the following pattern is common:

svn_error_t *inprocess_callback()
  return something;

So, if an error occurs, then all future cache calls deadlock; and it's
easy to forget to balance lock/unlock calls even by accident.


* Move to a with_cache_locked() callback paradigm, as already done in
  FSFS and libsvn_wc. This is harder to read but will encourage
  minimizing critical sections and will ensure that the cache is
  properly unlocked on non-fatal error conditions (i.e., those that
  don't correspond to a corrupted cache).

* Alternatively, add SVN_ERR_ASSERT(cache->is_locked) to relevant
  helper functions. This will ensure that locks are either cleared or
  (if stale) noisily complained about, but not deadlock.

* If body() discovers a fatal error condition... well, we could just
  SVN_ERR_ASSERT() out. Or we could set cache->malfunctioning := TRUE
  and then check that at all entry points.

* [ this is somewhat orthogonal ]

  The cache passes through (unmodified) all errors from the
  serialize/deserialize functions. Aren't them some conditions that
  we'd like those callbacks to be able to signal (to the cache or to
  the cache's user)? For example:


Barring objections I'll look into implementing something that eliminates
the potential deadlock.
Received on 2011-05-19 17:02:04 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.