Gentoo Archives: gentoo-portage-dev

From: Brian Dolbec <dolsen@g.o>
To: gentoo-portage-dev@l.g.o
Subject: Re: [gentoo-portage-dev] [PATCH] BinpkgFetcher: use async lock (bug 614110)
Date: Sat, 21 Apr 2018 22:07:39
Message-Id: 20180421150733.72dcd3cd@professor-x
In Reply to: [gentoo-portage-dev] [PATCH] BinpkgFetcher: use async lock (bug 614110) by Zac Medico
1 On Sat, 21 Apr 2018 12:27:28 -0700
2 Zac Medico <zmedico@g.o> wrote:
3
4 > In order to avoid event loop recursion, convert the
5 > _BinpkgFetcherProcess.lock() method to an async_lock
6 > method for use by BinpkgFetcher.
7 >
8 > Bug: https://bugs.gentoo.org/614110
9 > ---
10 > pym/_emerge/BinpkgFetcher.py | 53
11 > +++++++++++++++++++++++++++----------------- 1 file changed, 33
12 > insertions(+), 20 deletions(-)
13 >
14 > diff --git a/pym/_emerge/BinpkgFetcher.py
15 > b/pym/_emerge/BinpkgFetcher.py index 5ca7a45cf..2bbc0a26f 100644
16 > --- a/pym/_emerge/BinpkgFetcher.py
17 > +++ b/pym/_emerge/BinpkgFetcher.py
18 > @@ -32,11 +32,24 @@ class BinpkgFetcher(CompositeTask):
19 > pkg.cpv) + ".partial"
20 >
21 > def _start(self):
22 > - self._start_task(
23 > -
24 > _BinpkgFetcherProcess(background=self.background,
25 > - logfile=self.logfile, pkg=self.pkg,
26 > pkg_path=self.pkg_path,
27 > - pretend=self.pretend,
28 > scheduler=self.scheduler),
29 > - self._fetcher_exit)
30 > + fetcher =
31 > _BinpkgFetcherProcess(background=self.background,
32 > + logfile=self.logfile, pkg=self.pkg,
33 > pkg_path=self.pkg_path,
34 > + pretend=self.pretend,
35 > scheduler=self.scheduler) +
36 > + if not self.pretend:
37 > +
38 > portage.util.ensure_dirs(os.path.dirname(self.pkg_path))
39 > + if "distlocks" in
40 > self.pkg.root_config.settings.features:
41 > + self._start_task(
42 > +
43 > AsyncTaskFuture(future=fetcher.async_lock()),
44 > +
45 > functools.partial(self._start_locked, fetcher))
46 > + return
47 > +
48 > + self._start_task(fetcher, self._fetcher_exit)
49 > +
50 > + def _start_locked(self, fetcher, lock_task):
51 > + self._assert_current(lock_task)
52 > + lock_task.future.result()
53 > + self._start_task(fetcher, self._fetcher_exit)
54 >
55 > def _fetcher_exit(self, fetcher):
56 > self._assert_current(fetcher)
57 > @@ -68,13 +81,8 @@ class _BinpkgFetcherProcess(SpawnProcess):
58 > pretend = self.pretend
59 > bintree = pkg.root_config.trees["bintree"]
60 > settings = bintree.settings
61 > - use_locks = "distlocks" in settings.features
62 > pkg_path = self.pkg_path
63 >
64 > - if not pretend:
65 > -
66 > portage.util.ensure_dirs(os.path.dirname(pkg_path))
67 > - if use_locks:
68 > - self.lock()
69 > exists = os.path.exists(pkg_path)
70 > resume = exists and os.path.basename(pkg_path) in
71 > bintree.invalids if not (pretend or resume):
72 > @@ -184,7 +192,7 @@ class _BinpkgFetcherProcess(SpawnProcess):
73 > except
74 > OSError: pass
75 >
76 > - def lock(self):
77 > + def async_lock(self):
78 > """
79 > This raises an AlreadyLocked exception if lock() is
80 > called while a lock is already held. In order to avoid this, call
81 > @@ -194,17 +202,22 @@ class _BinpkgFetcherProcess(SpawnProcess):
82 > if self._lock_obj is not None:
83 > raise self.AlreadyLocked((self._lock_obj,))
84 >
85 > - async_lock = AsynchronousLock(path=self.pkg_path,
86 > - scheduler=self.scheduler)
87 > - async_lock.start()
88 > + result = self.scheduler.create_future()
89 >
90 > - if async_lock.wait() != os.EX_OK:
91 > - # TODO: Use CompositeTask for better
92 > handling, like in EbuildPhase.
93 > - raise AssertionError("AsynchronousLock
94 > failed with returncode %s" \
95 > - % (async_lock.returncode,))
96 > + def acquired_lock(async_lock):
97 > + if async_lock.wait() == os.EX_OK:
98 > + self.locked = True
99 > + result.set_result(None)
100 > + else:
101 > + result.set_exception(AssertionError(
102 > + "AsynchronousLock failed
103 > with returncode %s"
104 > + % (async_lock.returncode,)))
105 >
106 > - self._lock_obj = async_lock
107 > - self.locked = True
108 > + self._lock_obj = AsynchronousLock(path=self.pkg_path,
109 > + scheduler=self.scheduler)
110 > + self._lock_obj.addExitListener(acquired_lock)
111 > + self._lock_obj.start()
112 > + return result
113 >
114 > class AlreadyLocked(portage.exception.PortageException):
115 > pass
116
117
118 Looks fine to me :)
119 --
120 Brian Dolbec <dolsen>

Replies