[apt-proxy-devel] r641 - in trunk: apt_proxy apt_proxy/test debian

[Chris Halls]

Author: halls
Date: Wed Jan 24 13:25:03 2007
New Revision: 641

Modified:
   trunk/apt_proxy/cache.py
   trunk/apt_proxy/test/test_requests.py
   trunk/debian/changelog
Log:
Close a longstanding bug where clients would hang when receiving files from the cache. The problem was caused by reusing the same file handle for several requests at once.  (Closes: #274679, #382078, #322242, #397399, #397403, #398217)


Modified: trunk/apt_proxy/cache.py
==============================================================================
--- trunk/apt_proxy/cache.py	(original)
+++ trunk/apt_proxy/cache.py	Wed Jan 24 13:25:03 2007
@@ -296,14 +296,13 @@
             self.state = self.STATE_SENDFILE
             if size > 0:
                 log.debug("Sending file to clients:%s size:%s" % (self.file_path, size), 'CacheEntry')
-                if not self.streamfile: # Only open file if not already open
-                    self.streamfile = open(self.file_path,'rb')
-                #fcntl.lockf(file.fileno(), fcntl.LOCK_SH)
 
                 for request in self.requests:
                     if request.start_streaming(size, mtime):
                         log.debug("Streaming " + self.file_path)
-                        d = basic.FileSender().beginFileTransfer(self.streamfile, request)
+                        streamfile = open(self.file_path, 'rb')
+                        d = basic.FileSender().beginFileTransfer(streamfile, request)
+                        d.addBoth(lambda x: streamfile.close())
                         d.addCallback(self.file_transfer_complete, request, self.file_path)
                         d.addErrback(self.file_transfer_fail, request, self.file_path)
                     else:

Modified: trunk/apt_proxy/test/test_requests.py
==============================================================================
--- trunk/apt_proxy/test/test_requests.py	(original)
+++ trunk/apt_proxy/test/test_requests.py	Wed Jan 24 13:25:03 2007
@@ -305,6 +305,7 @@
         self.server = serverFactory()
         self.uriSuffix = uriSuffix
         self.port = self.server.start()
+        self.checkDest = True # Used to supress assertion when downloading the same file multiple times
 
         backend_uri = self.protocol + "://127.0.0.1:" + str(self.port)
         if self.uriSuffix:
@@ -347,8 +348,9 @@
         Then check that file does not exist
         """
         filename, sourcepath, destpath = self.calcFilePaths(file)
-        # File should not be in cache
-        self.assertRaises(OSError, os.stat, destpath)
+        if self.checkDest:
+            # File should not be in cache
+            self.assertRaises(OSError, os.stat, destpath)
         return filename, sourcepath, destpath
         
     def downloadFiles(self, *files):
@@ -554,6 +556,33 @@
         return d
     testConnectionRefused.timeout = 2
         
+    def testParallelFirstDownload(self):
+        """
+        same request from 3 clients (not in cache)
+        """
+        self.testResult = defer.Deferred()
+        d1 = self.downloadFile()
+        self.checkDest = False # Do not raise assertion when downloading the same file several times
+        d2 = self.downloadFile()
+        d3 = self.downloadFile()
+        dl = defer.DeferredList([d1,d2,d3])
+        #dl.addCallback(self.ParallelDownload2)
+        return dl
+    testParallelFirstDownload.timeout = 2
+        
+    def testParallelDownload(self):
+        self.testResult = defer.Deferred()
+        d = self.downloadFile(file='/packages/apt_0.0.1_test.deb')
+        d.addCallback(self.ParallelDownload2)
+        return self.testResult
+    testParallelDownload.timeout = 2
+    def ParallelDownload2(self,x):
+        log.debug("testParallelDownload: First download passed, now requesting cached file from 2 separate clients")
+        self.checkDest = False # Do not raise assertion when downloading the same file several times
+        d1 = self.downloadFile(file='/packages/apt_0.0.1_test.deb') # Using immutable file
+        d2 = self.downloadFile(file='/packages/apt_0.0.1_test.deb')
+        dl = defer.DeferredList([d1,d2])
+        dl.chainDeferred(self.testResult)
         
     #def testTimeout(self):
         #pass

Modified: trunk/debian/changelog
==============================================================================
--- trunk/debian/changelog	(original)
+++ trunk/debian/changelog	Wed Jan 24 13:25:03 2007
@@ -1,6 +1,11 @@
 apt-proxy (1.9.36~svn) unstable; urgency=low
 
   [ Chris Halls ]
+  * Acknowledge NMU by Steinar H. Gunderson, thanks! (Closes: #386344)
+  * Close a longstanding bug where clients would hang when receiving
+    files from the cache. The problem was caused by reusing the same
+    file handle for several requests at once.
+    (Closes: #274679, #382078, #322242, #397399, #397403, #398217)
   * Change the meaning of min_refresh_delay parameter, so the
     delay is measured from the modification time of the file on the
     backend instead of the time a client last requested this file.
@@ -15,14 +20,13 @@
     Thanks Jason Thomas for the patch (Closes: #348985)
   * Uncompress Packages.gz and Packages.bz2 on the fly, and
     update databases from these files (Closes: #TODO)
-  * Acknowledge NMU by Steinar H. Gunderson, thanks! (Closes: #386344)
 
   [ Mark Sheppard ]
   * Generate an error if a client attempts to retrieve
     http://server:9999/  (Closes: #386546)
   * When returning an error, generate an HTML page containing the error
 
- -- Chris Halls <chris.halls at credativ.co.uk>  Wed, 13 Dec 2006 22:33:24 +0100
+ -- Chris Halls <halls at debian.org>  Mon, 22 Jan 2007 18:23:04 +0000
 
 apt-proxy (1.9.35) unstable; urgency=low