Bug 69446 - Potential race when generating gtypes (and more)
Summary: Potential race when generating gtypes (and more)
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: mission-control (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL:
Whiteboard: review?
Keywords: patch
Depends on:
Blocks: 69600
  Show dependency treegraph
 
Reported: 2013-09-16 23:47 UTC by Ross Burton
Modified: 2014-02-03 18:14 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Patch (2.47 KB, patch)
2013-09-30 14:37 UTC, Ross Burton
Details | Splinter Review
codegen: avoid generating more than one thing in the same command (2.68 KB, patch)
2014-01-30 13:31 UTC, Simon McVittie
Details | Splinter Review

Description Ross Burton 2013-09-16 23:47:57 UTC
set_file_contents() uses a predictable name for it's temporary file when writing atomically, and (at least) mission-control can hit a race in parallel builds. (https://bugzilla.yoctoproject.org/show_bug.cgi?id=5184)

Luckily this is known as telepathy-glib fixed these in 36c2a545c9c1d1cc6db205bfc33d980d29b0a0f6 so the same fixes need to be applied to mission-control.

(fwiw, I've also a proof-of-concept patch for the predicable name issue)
Comment 1 Simon McVittie 2013-09-17 08:55:43 UTC
(In reply to comment #0)
> set_file_contents() uses a predictable name for it's temporary file when
> writing atomically, and (at least) mission-control can hit a race in
> parallel builds. (https://bugzilla.yoctoproject.org/show_bug.cgi?id=5184)
> 
> Luckily this is known as telepathy-glib fixed these in
> 36c2a545c9c1d1cc6db205bfc33d980d29b0a0f6 so the same fixes need to be
> applied to mission-control.

Indeed. I thought I'd fixed this in all projects, but apparently I missed MC.

> (fwiw, I've also a proof-of-concept patch for the predicable name issue)

I don't think this is really a problem: we're writing to a user-controlled directory, so we're not vulnerable to symlink attacks or anything. If we just avoided the "generate the same file twice" anti-pattern, it'd be fine.
Comment 2 Ross Burton 2013-09-30 14:37:40 UTC
Created attachment 86851 [details] [review]
Patch

This patch was sent to fix the issue in our builds but sadly the author didn't submit it here, so I'm doing it for them.
Comment 3 Simon McVittie 2014-01-30 13:31:12 UTC
Created attachment 93055 [details] [review]
codegen: avoid generating more than one thing in the same  command

As with telepathy-glib commit 36c2a545c9, a rule like this:

    _gen/x.c _gen/x.h: prerequisites
        $(AM_V_GEN)x-generator

doesn't consider x.c and x.h together. Instead, it expands to two rules,
one to generate x.c and one to generate x.h, which happen to run the
same commands.

This means that in the worst case, you can end up running x-generator
twice in parallel, and they'll race with each other and overwrite or
delete each other's output.

Updated version of a patch from Ross Burton, taking into account that
we now generate a separate header for the gtk-doc. I use the gtk-doc
header as the one that "matters", because it's the last to be generated.

---

I'm proposing this patch even though I'm about to delete the code-generation stuff, because (a) I just encountered it, and (b) I would like a `git revert` of that deletion to bring us back to a correct state.

Not using Ross' patch directly because we now generate more files than we used to.
Comment 4 Guillaume Desmottes 2014-02-03 11:24:28 UTC
Comment on attachment 93055 [details] [review]
codegen: avoid generating more than one thing in the same  command

Review of attachment 93055 [details] [review]:
-----------------------------------------------------------------

++
Comment 5 Simon McVittie 2014-02-03 16:27:11 UTC
Fixed in git for 5.17.0. Not fixed for 5.16 yet.
Comment 6 Simon McVittie 2014-02-03 18:14:36 UTC
Also 5.16.2


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.