[MERGE/RFC] no lock for 'is_shared'

John Arbash Meinel john at arbash-meinel.com
Fri Jul 25 14:01:21 BST 2008


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Clatworthy wrote:
| John Arbash Meinel wrote:
|> One of the things I noticed when tracing update, is that we end up
|> reading the pack-names file twice. And I believe it is because
|> 'is_shared' takes out an implicit write lock, which we then throw away.
|>
|> And we use 'is_shared()' because BzrDir.find_repository() needs to know
|> whether it can use the containing repository or not. And it is done at
|> an early enough stage that we aren't going to have the repository
|> permanently locked yet. (It is done early enough that we don't even know
|> if we want a read lock or a write lock.)
|>
|> So here is a possible patch:
|
| John,
|
| Can you post this as an attachment so BB can track it correctly?
|

I didn't really want to do the full commit/send/etc cycle for what is
mostly a drive-by fix. BB seems to be tracking it fine, and I'll just
make sure to manually mark it "merged" if/when appropriate.

|> It just removes the implicit lock. We aren't saving the value anyway, so
|> I don't think locking buys us anything but a round trip to read
|> something we aren't going to cache.
|
| From an API design perspective, it does feel like is_shared() ought
| to be locked, doesn't it? Perhaps we want a sister method
| (is_shared_local say?) that API clients not requiring locking could
| call instead?
|
| Ian C.
|

I'm not sure about something called "is_shared_local". I *can* say that
taking out a lock while *probing* for information about a repository
seems wrong.

Somewhat like if you took a lock to read the .bzr/repository/format file.

And certainly if pack repositories didn't automatically read pack-names
when you take a read-lock, I wouldn't have even noticed. But they do,
and I've already tried to go down that hole. (executive summary: it
complicates a lot of the other code, which has to see if it has been
loaded yet or not before continuing, rather than just always having it
loaded.)

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkiJzqEACgkQJdeBCYSNAAPjkACgoRKjt5gRoetXDBT3Z3Jrb/hS
iioAn30X3ywLzNhKdMEyq7amY86x/Qc9
=ZXHY
-----END PGP SIGNATURE-----



More information about the bazaar mailing list