Message ID | 20220801203418.57677-1-JPEWhacker@gmail.com |
---|---|
State | New |
Headers | show |
Series | [bitbake-devel] siggen: Fix sigtask data not being renamed atomically | expand |
Hmm, would it be better to os.chmod(fd, mode) rather than by filename, since it's still open? On Mon, Aug 1, 2022 at 1:34 PM Joshua Watt <JPEWhacker@gmail.com> wrote: > Signature generation uses mkstemp() to get a file descriptor to a unique > file and then write the signature into it, however it closed the file > before doing the chmod() and rename() operations. Closing the file means > that other mkstemp() could potentially open the same file and race with > the chmod() and rename(), causing a error. While it may not sound like > this would be very likely, glibc (at least) generates the filename for > mkstemp() using the system clock, meaning that it is much more likely > for highly parallel builds sharing sstate over NFS to encounter the race > condition. > > To fix the problem, perform the chmod() and rename() while the file is > still open, since this prevents other mkstemp() calls from being able to > open the file (due to the O_EXCL flag). > > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com> > --- > bitbake/lib/bb/siggen.py | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/bitbake/lib/bb/siggen.py b/bitbake/lib/bb/siggen.py > index 3f3d6df54d..55f25235df 100644 > --- a/bitbake/lib/bb/siggen.py > +++ b/bitbake/lib/bb/siggen.py > @@ -426,18 +426,18 @@ class SignatureGeneratorBasic(SignatureGenerator): > sigfile = sigfile.replace(self.taskhash[tid], > computed_taskhash) > > fd, tmpfile = tempfile.mkstemp(dir=os.path.dirname(sigfile), > prefix="sigtask.") > - try: > - with bb.compress.zstd.open(fd, "wt", encoding="utf-8", > num_threads=1) as f: > + with bb.compress.zstd.open(fd, "wt", encoding="utf-8", > num_threads=1) as f: > + try: > json.dump(data, f, sort_keys=True, separators=(",", ":"), > cls=SetEncoder) > f.flush() > - os.chmod(tmpfile, 0o664) > - bb.utils.rename(tmpfile, sigfile) > - except (OSError, IOError) as err: > - try: > - os.unlink(tmpfile) > - except OSError: > - pass > - raise err > + os.chmod(tmpfile, 0o664) > + bb.utils.rename(tmpfile, sigfile) > + except (OSError, IOError) as err: > + try: > + os.unlink(tmpfile) > + except OSError: > + pass > + raise err > > def dump_sigfn(self, fn, dataCaches, options): > if fn in self.taskdeps: > -- > 2.33.0 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#13862): > https://lists.openembedded.org/g/bitbake-devel/message/13862 > Mute This Topic: https://lists.openembedded.org/mt/92757646/3617123 > Group Owner: bitbake-devel+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [ > kergoth@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
diff --git a/bitbake/lib/bb/siggen.py b/bitbake/lib/bb/siggen.py index 3f3d6df54d..55f25235df 100644 --- a/bitbake/lib/bb/siggen.py +++ b/bitbake/lib/bb/siggen.py @@ -426,18 +426,18 @@ class SignatureGeneratorBasic(SignatureGenerator): sigfile = sigfile.replace(self.taskhash[tid], computed_taskhash) fd, tmpfile = tempfile.mkstemp(dir=os.path.dirname(sigfile), prefix="sigtask.") - try: - with bb.compress.zstd.open(fd, "wt", encoding="utf-8", num_threads=1) as f: + with bb.compress.zstd.open(fd, "wt", encoding="utf-8", num_threads=1) as f: + try: json.dump(data, f, sort_keys=True, separators=(",", ":"), cls=SetEncoder) f.flush() - os.chmod(tmpfile, 0o664) - bb.utils.rename(tmpfile, sigfile) - except (OSError, IOError) as err: - try: - os.unlink(tmpfile) - except OSError: - pass - raise err + os.chmod(tmpfile, 0o664) + bb.utils.rename(tmpfile, sigfile) + except (OSError, IOError) as err: + try: + os.unlink(tmpfile) + except OSError: + pass + raise err def dump_sigfn(self, fn, dataCaches, options): if fn in self.taskdeps:
Signature generation uses mkstemp() to get a file descriptor to a unique file and then write the signature into it, however it closed the file before doing the chmod() and rename() operations. Closing the file means that other mkstemp() could potentially open the same file and race with the chmod() and rename(), causing a error. While it may not sound like this would be very likely, glibc (at least) generates the filename for mkstemp() using the system clock, meaning that it is much more likely for highly parallel builds sharing sstate over NFS to encounter the race condition. To fix the problem, perform the chmod() and rename() while the file is still open, since this prevents other mkstemp() calls from being able to open the file (due to the O_EXCL flag). Signed-off-by: Joshua Watt <JPEWhacker@gmail.com> --- bitbake/lib/bb/siggen.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)