diff mbox series

[3/3] fetch/wget: mitigate a wget race condition when listing FTP directories

Message ID 20170725101719.2172-3-ross.burton@intel.com
State New
Headers show
Series [1/3] elfutils: use HTTP instead of FTP to fetch | expand

Commit Message

Ross Burton July 25, 2017, 10:17 a.m. UTC
When wget is fetching a listing for a directory over FTP it writes to a
temporary file called .listing in the current directory.  If there are many such
operations happening in parallel - for example during 'bitbake world -c
checkpkg' - then up to BB_NUMBER_THREADS instances of wget will be racing to
write to, read, and delete the same file.

This results in various failures such as the file disappearing before wget has
processed it or the file changing contents, which causes checkpkg to randomly
fail.

Mitigate the race condition by creating a temporary directory to run wget in
when doing directory listings.

[ YOCTO #11828 ]

Signed-off-by: Ross Burton <ross.burton@intel.com>

---
 bitbake/lib/bb/fetch2/wget.py | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

-- 
2.11.0

-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Comments

Ross Burton July 25, 2017, 10:17 a.m. UTC | #1
Wrong list, sorry.

Ross

On 25 July 2017 at 11:17, Ross Burton <ross.burton@intel.com> wrote:

> When wget is fetching a listing for a directory over FTP it writes to a

> temporary file called .listing in the current directory.  If there are

> many such

> operations happening in parallel - for example during 'bitbake world -c

> checkpkg' - then up to BB_NUMBER_THREADS instances of wget will be racing

> to

> write to, read, and delete the same file.

>

> This results in various failures such as the file disappearing before wget

> has

> processed it or the file changing contents, which causes checkpkg to

> randomly

> fail.

>

> Mitigate the race condition by creating a temporary directory to run wget

> in

> when doing directory listings.

>

> [ YOCTO #11828 ]

>

> Signed-off-by: Ross Burton <ross.burton@intel.com>

> ---

>  bitbake/lib/bb/fetch2/wget.py | 23 +++++++++++------------

>  1 file changed, 11 insertions(+), 12 deletions(-)

>

> diff --git a/bitbake/lib/bb/fetch2/wget.py b/bitbake/lib/bb/fetch2/wget.py

> index 208ee9bdd69..8ee9769d39b 100644

> --- a/bitbake/lib/bb/fetch2/wget.py

> +++ b/bitbake/lib/bb/fetch2/wget.py

> @@ -90,13 +90,13 @@ class Wget(FetchMethod):

>

>          self.basecmd = d.getVar("FETCHCMD_wget") or "/usr/bin/env wget -t

> 2 -T 30 --passive-ftp --no-check-certificate"

>

> -    def _runwget(self, ud, d, command, quiet):

> +    def _runwget(self, ud, d, command, quiet, workdir=None):

>

>          progresshandler = WgetProgressHandler(d)

>

>          logger.debug(2, "Fetching %s using command '%s'" % (ud.url,

> command))

>          bb.fetch2.check_network_access(d, command, ud.url)

> -        runfetchcmd(command + ' --progress=dot -v', d, quiet,

> log=progresshandler)

> +        runfetchcmd(command + ' --progress=dot -v', d, quiet,

> log=progresshandler, workdir=workdir)

>

>      def download(self, ud, d):

>          """Fetch urls"""

> @@ -422,17 +422,16 @@ class Wget(FetchMethod):

>          Run fetch checkstatus to get directory information

>          """

>          f = tempfile.NamedTemporaryFile()

> +        with tempfile.TemporaryDirectory(prefix="wget-index-") as

> workdir, tempfile.NamedTemporaryFile(dir=workdir, prefix="wget-listing-")

> as f:

> +            agent = "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.12)

> Gecko/20101027 Ubuntu/9.10 (karmic) Firefox/3.6.12"

> +            fetchcmd = self.basecmd

> +            fetchcmd += " -O " + f.name + " --user-agent='" + agent + "'

> '" + uri + "'"

> +            try:

> +                self._runwget(ud, d, fetchcmd, True, workdir=workdir)

> +                fetchresult = f.read()

> +            except bb.fetch2.BBFetchException:

> +                fetchresult = ""

>

> -        agent = "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.12)

> Gecko/20101027 Ubuntu/9.10 (karmic) Firefox/3.6.12"

> -        fetchcmd = self.basecmd

> -        fetchcmd += " -O " + f.name + " --user-agent='" + agent + "' '"

> + uri + "'"

> -        try:

> -            self._runwget(ud, d, fetchcmd, True)

> -            fetchresult = f.read()

> -        except bb.fetch2.BBFetchException:

> -            fetchresult = ""

> -

> -        f.close()

>          return fetchresult

>

>      def _check_latest_version(self, url, package, package_regex,

> current_version, ud, d):

> --

> 2.11.0

>

>
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
diff mbox series

Patch

diff --git a/bitbake/lib/bb/fetch2/wget.py b/bitbake/lib/bb/fetch2/wget.py
index 208ee9bdd69..8ee9769d39b 100644
--- a/bitbake/lib/bb/fetch2/wget.py
+++ b/bitbake/lib/bb/fetch2/wget.py
@@ -90,13 +90,13 @@  class Wget(FetchMethod):
 
         self.basecmd = d.getVar("FETCHCMD_wget") or "/usr/bin/env wget -t 2 -T 30 --passive-ftp --no-check-certificate"
 
-    def _runwget(self, ud, d, command, quiet):
+    def _runwget(self, ud, d, command, quiet, workdir=None):
 
         progresshandler = WgetProgressHandler(d)
 
         logger.debug(2, "Fetching %s using command '%s'" % (ud.url, command))
         bb.fetch2.check_network_access(d, command, ud.url)
-        runfetchcmd(command + ' --progress=dot -v', d, quiet, log=progresshandler)
+        runfetchcmd(command + ' --progress=dot -v', d, quiet, log=progresshandler, workdir=workdir)
 
     def download(self, ud, d):
         """Fetch urls"""
@@ -422,17 +422,16 @@  class Wget(FetchMethod):
         Run fetch checkstatus to get directory information
         """
         f = tempfile.NamedTemporaryFile()
+        with tempfile.TemporaryDirectory(prefix="wget-index-") as workdir, tempfile.NamedTemporaryFile(dir=workdir, prefix="wget-listing-") as f:
+            agent = "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.12) Gecko/20101027 Ubuntu/9.10 (karmic) Firefox/3.6.12"
+            fetchcmd = self.basecmd
+            fetchcmd += " -O " + f.name + " --user-agent='" + agent + "' '" + uri + "'"
+            try:
+                self._runwget(ud, d, fetchcmd, True, workdir=workdir)
+                fetchresult = f.read()
+            except bb.fetch2.BBFetchException:
+                fetchresult = ""
 
-        agent = "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.12) Gecko/20101027 Ubuntu/9.10 (karmic) Firefox/3.6.12"
-        fetchcmd = self.basecmd
-        fetchcmd += " -O " + f.name + " --user-agent='" + agent + "' '" + uri + "'"
-        try:
-            self._runwget(ud, d, fetchcmd, True)
-            fetchresult = f.read()
-        except bb.fetch2.BBFetchException:
-            fetchresult = ""
-
-        f.close()
         return fetchresult
 
     def _check_latest_version(self, url, package, package_regex, current_version, ud, d):