CVS Import Bug Detected: - Please Respond

[Oproescu Bogdan (KTXA 3)]

   Hi Derek, 

   Thanks for your message, and congratulations, you were the first person 
   so spot the error in the source code of CVS, and tell us that CVS omits 
   locking for the "cvs import" command. 

   I like the 2-pass solution you described below: it's self-contained 
   in the "cvs import" command, and does not require additional auxiliary 
   Files to be added to the Module that can act as CVS Semaphores. 

   I know you have no obligations to us or the CVS Open Source Community 
   to implement the patch you described to solve this, but I would like 
   to ask you to try to implement this for us at your earliest convenience, 
   or eventually ask one of your colleagues who is authorized to make changes 
   to the CVS Source Code to implement this for us. 

   Currently, to avoid this daily file corruption on our CVS Central Server, 
   we are forced to implement awkward workarounds that circumvent this 
   situation, because we have over 500 CVS Users in a large project here 
   that are affected by this file corruption issue. 
   
   Therefore, if you are able to implement this patch for us and copy it 
   to the www.cvshome.org downloads page, we would be very grateful, and 
   I am sure that our CVS Users above will restore their confidence in 
   CVS in knowing that it performs the necessary locking for cvs import. 
   
   I am surprised that no one observed this non-locking behaviour 
   for CVS until now, as a lot of people use CVS, but I hope that 
   with your cooperation we will quickly fix this issue. 

   Cheers & Regards, Bogdan 

   Bogdan Oproescu   

   CREDIT SUISSE FINANCIAL SERVICES
   Technology & Services
   Advanced Middleware &
   Development Environments KTXA 3 
   Postfach 600
   CH-8070 Zürich
   Tel.: +41 1 334 6846
   Fax.:+41 1 332 8024
   E-Mail: bogdan.oproescu@credit-suisse.ch
   Internet: http://www.credit-suisse.ch/de/index.html

-----Original Message-----
From: Derek Price [mailto:derek@ximbiot.com] 
Sent: Wednesday, August 10, 2005 9:22 PM
To: Oproescu Bogdan (KTXA 3)
Cc: 'Bernd Jendrissek'; 'bug-cvs@nongnu.org'
Subject: Re: Next Steps: CVS Import Bug - Please Respond

Oproescu Bogdan (KTXA 3) wrote:

> [Clueless guessing here...]  Are you able to correlate the error
> messages with accesses to perhaps the same file from a different client?
> If you suspect a race between two rename_file()s, one could well imagine
> a first call to succeed, and a subsequent to fail when the source file
> is already "renamed away".  ???
>
>     I mentioned earlier that I can correlate the error messages with
>     accesses ( more specifically, "cvs import"s ) to the SAME Module
>     and Repository pair on our CVS Central Server. The question is
>     the following: since "import" is not atomic, there is no lock
>     created for some Module when imported, so another parallel "cvs
> import"
>     on the SAME Module & Repository pair would conceivably corrupt the
> .jar file.


"Not atomic" is *not* equivalent to "non-locking".  In fact, CVS locks
the entire directory tree for write before making any changes to archive
files on commit, though commits are not atomic.

Unfortunately, looking at the code, it does look like CVS actually
foregoes locking for import!  Anyone know why?

It shouldn't be too hard to fix.  In the import function in import.c,
the import_descend and import_descend_dir need to be split into two
passes.  The first pass should construct the directory strucure in the
repository and save a list of the directory names.  It's mostly going to
fit right into what those functions currently do in client mode, though
the global repository variable may need to be deglobalized.  The list of
files and directories to be imported could be cached at the same time,
avoiding some of the overhead of the extra pass, but that's not necessary.

Then, the directory list needs to be used as an argument to Write_Lock
(or lock_list_promotably for feature) when not on the client, just
before the import data is traversed for the second time and actually
imported.

During the second pass, as each directory is visited, the write lock
will need to be grabbed on feature.  On both feature and stable the
write lock will need to be released when each directory is finished with.

I don't have time to implement this myself just now, but I'd love to see
a patch!

Regards,

Derek
-- 
Derek R. Price
CVS Solutions Architect
Ximbiot <http://ximbiot.com>
v: +1 717.579.6168
f: +1 717.234.3125
<mailto:derek@ximbiot.com>