Gareth White wrote:
> That does sound like a good way to go as it would actually solve two
> issues (the original bug, plus prematurely deleting obsoleted files).
>
> Would you recommend adding a "packs_to_obsolete" member to this class?
> That would make it easy for _clear_obsolete_packs() to know which ones
> to skip deletion of, while at the same time allowing it to remove them
> from the list so they don't try to be obsoleted twice. An alternative is
> to try to pass in and return a list via _save_pack_names() but that
> seems a little messy.
>
> As an aside, I'm a bit unclear why _clear_obsolete_packs() is called
> from within _save_pack_names(). This does mean that files in
> "obsolete_packs" are deleted only while pack-names is locked, yet
> _obsolete_packs() moves files into there while it's not locked. I'm sure
> there must be a good reason - if not, perhaps we could do the deletions
> closer to where we obsolete the files. That would make it easier to
> implement this bugfix.
>
Once pack-names has been written, those files are no longer referenced,
so we can rename them from 'packs' to 'obsolete_packs' at our leisure
(we don't need the lock).
We probably don't need the lock when deleting content from
obsolete_packs either, as it doesn't effect the correctness of the
operation.
As for a member variable... you also could just mutate the list that is
passed in.
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Gareth White wrote: obsolete_ packs() to know which ones obsolete_ packs() is called
> That does sound like a good way to go as it would actually solve two
> issues (the original bug, plus prematurely deleting obsoleted files).
>
> Would you recommend adding a "packs_to_obsolete" member to this class?
> That would make it easy for _clear_
> to skip deletion of, while at the same time allowing it to remove them
> from the list so they don't try to be obsoleted twice. An alternative is
> to try to pass in and return a list via _save_pack_names() but that
> seems a little messy.
>
> As an aside, I'm a bit unclear why _clear_
> from within _save_pack_names(). This does mean that files in
> "obsolete_packs" are deleted only while pack-names is locked, yet
> _obsolete_packs() moves files into there while it's not locked. I'm sure
> there must be a good reason - if not, perhaps we could do the deletions
> closer to where we obsolete the files. That would make it easier to
> implement this bugfix.
>
Once pack-names has been written, those files are no longer referenced,
so we can rename them from 'packs' to 'obsolete_packs' at our leisure
(we don't need the lock).
We probably don't need the lock when deleting content from
obsolete_packs either, as it doesn't effect the correctness of the
operation.
As for a member variable... you also could just mutate the list that is
passed in.
John enigmail. mozdev. org/
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAkt WHkYACgkQJdeBCY SNAAMJDQCfaMC1T A/3ZeXhLC4emZlw cjxL cIb30BmVoOjP0sl 8M
QCoAn0tdtVFN3Co
=beA9
-----END PGP SIGNATURE-----