Patchwork [1/1] telepathy-mission-control: do_compile failed (race issue)

login
register
mail settings
Submitter Robert Yang
Date Sept. 14, 2013, 8:49 a.m.
Message ID <3b4c037e41f6678845fb29d97aae5f48f07a68c7.1379148332.git.liezhi.yang@windriver.com>
Download mbox | patch
Permalink /patch/58043/
State New
Headers show

Comments

Robert Yang - Sept. 14, 2013, 8:49 a.m.
There might be an error when parallel build:

[snip]
Traceback (most recent call last):
  File "/path/to/tools/glib-gtypes-generator.py", line 304, in <module>
    GTypesGenerator(dom, argv[1], argv[2])()
  File "/path/to/tools/glib-gtypes-generator.py", line 295, in __call__
    file_set_contents(self.output + '.h', ''.join(self.header))
  File "/path/to/tools/libtpcodegen.py", line 42, in file_set_contents
    os.rename(filename + '.tmp', filename)
OSError: [Errno 2] No such file or directory
[snip]

This is a race issue, the _gen/gtypes.h and _gen/gtypes-body.h may
write(remove/rename) _gen/gtypes.tmp at the same time, then there would
be the error.

Use ".NOTPARALLEL: _gen/gtypes.h _gen/gtypes-body.h" will fix the
problem.

[YOCTO #5184]

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 .../tmc-Makefile-fix-race.patch                    |   47 ++++++++++++++++++++
 .../telepathy/telepathy-mission-control_5.15.0.bb  |    5 ++-
 2 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-connectivity/telepathy/telepathy-mission-control/tmc-Makefile-fix-race.patch
Ross Burton - Sept. 16, 2013, 11:08 p.m.
On 14 September 2013 09:49, Robert Yang <liezhi.yang@windriver.com> wrote:
> There might be an error when parallel build:
>
> [snip]
> Traceback (most recent call last):
>   File "/path/to/tools/glib-gtypes-generator.py", line 304, in <module>
>     GTypesGenerator(dom, argv[1], argv[2])()
>   File "/path/to/tools/glib-gtypes-generator.py", line 295, in __call__
>     file_set_contents(self.output + '.h', ''.join(self.header))
>   File "/path/to/tools/libtpcodegen.py", line 42, in file_set_contents
>     os.rename(filename + '.tmp', filename)
> OSError: [Errno 2] No such file or directory
> [snip]
>
> This is a race issue, the _gen/gtypes.h and _gen/gtypes-body.h may
> write(remove/rename) _gen/gtypes.tmp at the same time, then there would
> be the error.
>
> Use ".NOTPARALLEL: _gen/gtypes.h _gen/gtypes-body.h" will fix the
> problem.

This is going to be a game of whack-a-mole as the root cause is the
bad temporary filename assignment in libtpcodegen.py.  Is there an
upstream bug for this?

Ross
Ross Burton - Sept. 16, 2013, 11:49 p.m.
On 17 September 2013 00:08, Burton, Ross <ross.burton@intel.com> wrote:
>> This is a race issue, the _gen/gtypes.h and _gen/gtypes-body.h may
>> write(remove/rename) _gen/gtypes.tmp at the same time, then there would
>> be the error.
>>
>> Use ".NOTPARALLEL: _gen/gtypes.h _gen/gtypes-body.h" will fix the
>> problem.
>
> This is going to be a game of whack-a-mole as the root cause is the
> bad temporary filename assignment in libtpcodegen.py.  Is there an
> upstream bug for this?

Had a quick look - there's a trivial race in the utility code that a
bit of tempfile can fix, but the more correct fix for
telepathy-mission-control is to do the same fix that telepathy-glib
did here:

http://cgit.freedesktop.org/telepathy/telepathy-glib/commit/?id=36c2a545c9c1d1cc6db205bfc33d980d29b0a0f6

https://bugs.freedesktop.org/show_bug.cgi?id=69446 is the upstream bug
I just filed for this.

Ross
Robert Yang - Sept. 17, 2013, 2:03 a.m.
On 09/17/2013 07:49 AM, Burton, Ross wrote:
> On 17 September 2013 00:08, Burton, Ross <ross.burton@intel.com> wrote:
>>> This is a race issue, the _gen/gtypes.h and _gen/gtypes-body.h may
>>> write(remove/rename) _gen/gtypes.tmp at the same time, then there would
>>> be the error.
>>>
>>> Use ".NOTPARALLEL: _gen/gtypes.h _gen/gtypes-body.h" will fix the
>>> problem.
>>
>> This is going to be a game of whack-a-mole as the root cause is the
>> bad temporary filename assignment in libtpcodegen.py.  Is there an
>> upstream bug for this?
>
> Had a quick look - there's a trivial race in the utility code that a
> bit of tempfile can fix, but the more correct fix for
> telepathy-mission-control is to do the same fix that telepathy-glib
> did here:
>
> http://cgit.freedesktop.org/telepathy/telepathy-glib/commit/?id=36c2a545c9c1d1cc6db205bfc33d980d29b0a0f6

I don't think that the commit you mentioned is a correct fix, he changed:

gen/gtypes.h _gen/gtypes-body.h: _gen/stable-spec.xml

To:

gen/gtypes.h: _gen/gtypes-body.h
	@:
gen/gtypes-body.h: _gen/stable-spec.xml

Now if we run "make gen/gtypes.h gen/gtypes-body.h", the gen/gtypes-body.h
will only run once, this is incorrect. And this fix has been dropped in the
up to date source.

// Robert

>
> https://bugs.freedesktop.org/show_bug.cgi?id=69446 is the upstream bug
> I just filed for this.
>
> Ross
>
>
Ross Burton - Sept. 17, 2013, 11:12 a.m.
On 17 September 2013 03:03, Robert Yang <liezhi.yang@windriver.com> wrote:
> I don't think that the commit you mentioned is a correct fix, he changed:
>
> gen/gtypes.h _gen/gtypes-body.h: _gen/stable-spec.xml
>
> To:
>
> gen/gtypes.h: _gen/gtypes-body.h
>         @:
> gen/gtypes-body.h: _gen/stable-spec.xml
>
> Now if we run "make gen/gtypes.h gen/gtypes-body.h", the gen/gtypes-body.h
> will only run once, this is incorrect.

But the Python script used in the second rule generates *both* files,
which is why the current rules in telepathy-mission-control are
incorrect:

~/Local/mess/37/telepathy-mission-control/src
$ ls _gen/gtypes*
_gen/gtypes-body.h  _gen/gtypes.h

~/Local/mess/37/telepathy-mission-control/src
$ rm _gen/gtypes* -f

~/Local/mess/37/telepathy-mission-control/src
$ make _gen/gtypes.h
/usr/bin/python ../tools/glib-gtypes-generator.py \
_gen/mcd.xml _gen/gtypes mc

~/Local/mess/37/telepathy-mission-control/src
$ ls _gen/gtypes*
_gen/gtypes-body.h  _gen/gtypes.h

The anti-pattern of:

foo bar:
  this-script-generates-both-files.sh

is wrong in parallel builds.

Ross
Robert Yang - Sept. 18, 2013, 2:45 a.m.
On 09/17/2013 07:12 PM, Burton, Ross wrote:
> On 17 September 2013 03:03, Robert Yang <liezhi.yang@windriver.com> wrote:
>> I don't think that the commit you mentioned is a correct fix, he changed:
>>
>> gen/gtypes.h _gen/gtypes-body.h: _gen/stable-spec.xml
>>
>> To:
>>
>> gen/gtypes.h: _gen/gtypes-body.h
>>          @:
>> gen/gtypes-body.h: _gen/stable-spec.xml
>>
>> Now if we run "make gen/gtypes.h gen/gtypes-body.h", the gen/gtypes-body.h
>> will only run once, this is incorrect.
>
> But the Python script used in the second rule generates *both* files,

Ah, yes, thanks, what's your opinion on my patch ? Is it OK or should it be
re-worked ? I think that this fix is simple and easy to understand.

// Robert

> which is why the current rules in telepathy-mission-control are
> incorrect:
>
> ~/Local/mess/37/telepathy-mission-control/src
> $ ls _gen/gtypes*
> _gen/gtypes-body.h  _gen/gtypes.h
>
> ~/Local/mess/37/telepathy-mission-control/src
> $ rm _gen/gtypes* -f
>
> ~/Local/mess/37/telepathy-mission-control/src
> $ make _gen/gtypes.h
> /usr/bin/python ../tools/glib-gtypes-generator.py \
> _gen/mcd.xml _gen/gtypes mc
>
> ~/Local/mess/37/telepathy-mission-control/src
> $ ls _gen/gtypes*
> _gen/gtypes-body.h  _gen/gtypes.h
>
> The anti-pattern of:
>
> foo bar:
>    this-script-generates-both-files.sh
>
> is wrong in parallel builds.
>
> Ross
>
>
Ross Burton - Sept. 18, 2013, 6:24 a.m.
On 18 September 2013 03:45, Robert Yang <liezhi.yang@windriver.com> wrote:
> Ah, yes, thanks, what's your opinion on my patch ? Is it OK or should it be
> re-worked ? I think that this fix is simple and easy to understand.

It should be re-worked to use the pattern in the commit I mentioned -
using .NOTPARALLEL is working around the problem that the rule
dependencies are incorrect.

Ross

Patch

diff --git a/meta/recipes-connectivity/telepathy/telepathy-mission-control/tmc-Makefile-fix-race.patch b/meta/recipes-connectivity/telepathy/telepathy-mission-control/tmc-Makefile-fix-race.patch
new file mode 100644
index 0000000..537e1d7
--- /dev/null
+++ b/meta/recipes-connectivity/telepathy/telepathy-mission-control/tmc-Makefile-fix-race.patch
@@ -0,0 +1,47 @@ 
+From 029fcd600707f25a73b0171497140dab264f56b7 Mon Sep 17 00:00:00 2001
+From: Robert Yang <liezhi.yang@windriver.com>
+Date: Sat, 14 Sep 2013 04:26:46 -0400
+Subject: [PATCH] src/Makefile.am: let _gen/gtypes.h and _gen/gtypes-body.h run serially
+
+There might be an error when parallel build:
+
+[snip]
+Traceback (most recent call last):
+  File "/path/to/tools/glib-gtypes-generator.py", line 304, in <module>
+    GTypesGenerator(dom, argv[1], argv[2])()
+  File "/path/to/tools/glib-gtypes-generator.py", line 295, in __call__
+    file_set_contents(self.output + '.h', ''.join(self.header))
+  File "/path/to/tools/libtpcodegen.py", line 42, in file_set_contents
+    os.rename(filename + '.tmp', filename)
+OSError: [Errno 2] No such file or directory
+[snip]
+
+This is a race issue, the _gen/gtypes.h and _gen/gtypes-body.h may
+write(remove/rename) _gen/gtypes.tmp at the same time, then there would
+be the error.
+
+Use ".NOTPARALLEL: _gen/gtypes.h _gen/gtypes-body.h" will fix the
+problem.
+
+Upstream-Status: Pending
+
+Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
+---
+ src/Makefile.am |    2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/Makefile.am b/src/Makefile.am
+--- a/src/Makefile.am
++++ b/src/Makefile.am
+@@ -293,7 +293,7 @@ _gen/gtypes.h _gen/gtypes-body.h: _gen/mcd.xml \
+ 	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/tools/glib-gtypes-generator.py \
+ 		$< _gen/gtypes mc
+ 
+-
++.NOTPARALLEL: _gen/gtypes.h _gen/gtypes-body.h
+ 
+ # Generated files which must be generated per "class".
+ # (Currently the only "class" is nmc4, but the new API will need "classes"
+-- 
+1.7.10.4
+
diff --git a/meta/recipes-connectivity/telepathy/telepathy-mission-control_5.15.0.bb b/meta/recipes-connectivity/telepathy/telepathy-mission-control_5.15.0.bb
index b2736ca..91159e1 100644
--- a/meta/recipes-connectivity/telepathy/telepathy-mission-control_5.15.0.bb
+++ b/meta/recipes-connectivity/telepathy/telepathy-mission-control_5.15.0.bb
@@ -6,7 +6,10 @@  LIC_FILES_CHKSUM = "file://COPYING;md5=d8045f3b8f929c1cb29a1e3fd737b499 \
 SECTION = "libs"
 DEPENDS = "libtelepathy dbus-glib gconf libxslt-native"
 
-SRC_URI = "http://telepathy.freedesktop.org/releases/telepathy-mission-control/telepathy-mission-control-${PV}.tar.gz"
+SRC_URI = "http://telepathy.freedesktop.org/releases/telepathy-mission-control/telepathy-mission-control-${PV}.tar.gz \
+	   file://tmc-Makefile-fix-race.patch \
+	"
+
 SRC_URI[md5sum] = "0efd7d9043f6e189e8ed8111d043b52c"
 SRC_URI[sha256sum] = "88800d1b5f385e94b34721319ff4fe65c493632954d7229b36e57af5b4a7e1b5"