All of lore.kernel.org
 help / color / mirror / Atom feed
* [meta-oe][PATCH] protobuf-c: fix race condition
@ 2019-09-29  9:29 Chen Qi
  2019-10-17 20:44 ` Martin Jansa
  0 siblings, 1 reply; 9+ messages in thread
From: Chen Qi @ 2019-09-29  9:29 UTC (permalink / raw)
  To: openembedded-devel

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 .../0001-avoid-race-condition.patch           | 36 +++++++++++++++++++
 .../protobuf/protobuf-c_1.3.2.bb              |  1 +
 2 files changed, 37 insertions(+)
 create mode 100644 meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch

diff --git a/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
new file mode 100644
index 000000000..4fc7703d8
--- /dev/null
+++ b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
@@ -0,0 +1,36 @@
+From 216e31260b618ec73862f9f5336597f391444dac Mon Sep 17 00:00:00 2001
+From: Chen Qi <Qi.Chen@windriver.com>
+Date: Sun, 29 Sep 2019 17:20:42 +0800
+Subject: [PATCH] avoid race condition
+
+It's possible that the cxx-generate-packed-data.cc is compiled
+while the t/test-full.pb.h is being generated. This will result
+the following error.
+
+  DEBUG:	./t/test-full.pb.h:4:0: error: unterminated #ifndef
+  ./t/test-full.pb.h:4:0: error: unterminated #ifndef
+
+Add a dependency to avoid such problem.
+
+Upstream-Status: Pending
+
+Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
+---
+ Makefile.am | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/Makefile.am b/Makefile.am
+index b0cb065..1608ae0 100644
+--- a/Makefile.am
++++ b/Makefile.am
+@@ -156,6 +156,7 @@ noinst_PROGRAMS += \
+ t_generated_code2_cxx_generate_packed_data_SOURCES = \
+ 	t/generated-code2/cxx-generate-packed-data.cc \
+ 	t/test-full.pb.cc
++t/generated-code2/cxx-generate-packed-data.cc: t/test-full.pb.h
+ $(t_generated_code2_cxx_generate_packed_data_OBJECTS): t/test-full.pb.h
+ t_generated_code2_cxx_generate_packed_data_CXXFLAGS = \
+ 	$(AM_CXXFLAGS) \
+-- 
+2.17.1
+
diff --git a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
index 6d1ffc3f4..b92f82dec 100644
--- a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
+++ b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
@@ -15,6 +15,7 @@ DEPENDS = "protobuf-native protobuf"
 SRCREV = "1390409f4ee4e26d0635310995b516eb702c3f9e"
 
 SRC_URI = "git://github.com/protobuf-c/protobuf-c.git \
+           file://0001-avoid-race-condition.patch \
           "
 
 S = "${WORKDIR}/git"
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] protobuf-c: fix race condition
  2019-09-29  9:29 [meta-oe][PATCH] protobuf-c: fix race condition Chen Qi
@ 2019-10-17 20:44 ` Martin Jansa
  2019-10-17 22:11   ` Martin Jansa
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Jansa @ 2019-10-17 20:44 UTC (permalink / raw)
  To: Chen Qi; +Cc: openembedded-devel

[-- Attachment #1: Type: text/plain, Size: 3210 bytes --]

On Sun, Sep 29, 2019 at 05:29:56PM +0800, Chen Qi wrote:
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>  .../0001-avoid-race-condition.patch           | 36 +++++++++++++++++++
>  .../protobuf/protobuf-c_1.3.2.bb              |  1 +
>  2 files changed, 37 insertions(+)
>  create mode 100644 meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> 
> diff --git a/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> new file mode 100644
> index 000000000..4fc7703d8
> --- /dev/null
> +++ b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> @@ -0,0 +1,36 @@
> +From 216e31260b618ec73862f9f5336597f391444dac Mon Sep 17 00:00:00 2001
> +From: Chen Qi <Qi.Chen@windriver.com>
> +Date: Sun, 29 Sep 2019 17:20:42 +0800
> +Subject: [PATCH] avoid race condition
> +
> +It's possible that the cxx-generate-packed-data.cc is compiled
> +while the t/test-full.pb.h is being generated. This will result
> +the following error.
> +
> +  DEBUG:	./t/test-full.pb.h:4:0: error: unterminated #ifndef
> +  ./t/test-full.pb.h:4:0: error: unterminated #ifndef
> +
> +Add a dependency to avoid such problem.
> +
> +Upstream-Status: Pending

Thanks for this patch, it seems to indeed fix this issue, I've reported
a while ago:
https://www.mail-archive.com/openembedded-devel@lists.openembedded.org/msg64363.html

I think the upstream would be interested in this fix as well, see:
https://github.com/protobuf-c/protobuf-c/commit/b0bb56303366e2c072ee38eb5f1f11251b07239c

Cheers,

> +
> +Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> +---
> + Makefile.am | 1 +
> + 1 file changed, 1 insertion(+)
> +
> +diff --git a/Makefile.am b/Makefile.am
> +index b0cb065..1608ae0 100644
> +--- a/Makefile.am
> ++++ b/Makefile.am
> +@@ -156,6 +156,7 @@ noinst_PROGRAMS += \
> + t_generated_code2_cxx_generate_packed_data_SOURCES = \
> + 	t/generated-code2/cxx-generate-packed-data.cc \
> + 	t/test-full.pb.cc
> ++t/generated-code2/cxx-generate-packed-data.cc: t/test-full.pb.h
> + $(t_generated_code2_cxx_generate_packed_data_OBJECTS): t/test-full.pb.h
> + t_generated_code2_cxx_generate_packed_data_CXXFLAGS = \
> + 	$(AM_CXXFLAGS) \
> +-- 
> +2.17.1
> +
> diff --git a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> index 6d1ffc3f4..b92f82dec 100644
> --- a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> +++ b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> @@ -15,6 +15,7 @@ DEPENDS = "protobuf-native protobuf"
>  SRCREV = "1390409f4ee4e26d0635310995b516eb702c3f9e"
>  
>  SRC_URI = "git://github.com/protobuf-c/protobuf-c.git \
> +           file://0001-avoid-race-condition.patch \
>            "
>  
>  S = "${WORKDIR}/git"
> -- 
> 2.17.1
> 
> -- 
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-devel

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 201 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] protobuf-c: fix race condition
  2019-10-17 20:44 ` Martin Jansa
@ 2019-10-17 22:11   ` Martin Jansa
  2019-10-18  1:15     ` Sinan Kaya
  2019-10-18  9:07     ` Khem Raj
  0 siblings, 2 replies; 9+ messages in thread
From: Martin Jansa @ 2019-10-17 22:11 UTC (permalink / raw)
  To: Chen Qi; +Cc: openembedded-devel

Hmm I spoke too soon, at least rebuilding protobuf-c-native with this
change backported to thud still failed once with:

../git/t/generated-code2/cxx-generate-packed-data.cc:28:17: error: ‘foo’ is
not a namespace-name
 using namespace foo;
                 ^~~
../git/t/generated-code2/cxx-generate-packed-data.cc:28:20: error: expected
namespace-name before ‘;’ token
 using namespace foo;
                    ^
../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
‘TestEnumSmall’ does not name a type
 #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
                                    ^
../git/t/generated-code2/common-test-arrays.h:44:1: note: in expansion of
macro ‘TEST_ENUM_SMALL_TYPE_NAME’
 TEST_ENUM_SMALL_TYPE_NAME enum_small_0[] = { TEST_ENUM_SMALL(VALUE) };
 ^~~~~~~~~~~~~~~~~~~~~~~~~
../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
‘TestEnumSmall’ does not name a type
 #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
                                    ^
../git/t/generated-code2/common-test-arrays.h:45:1: note: in expansion of
macro ‘TEST_ENUM_SMALL_TYPE_NAME’
 TEST_ENUM_SMALL_TYPE_NAME enum_small_1[] = { TEST_ENUM_SMALL(OTHER_VALUE)
};
 ^~~~~~~~~~~~~~~~~~~~~~~~~
../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
‘TestEnumSmall’ does not name a type
 #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
                                    ^
../git/t/generated-code2/common-test-arrays.h:47:1: note: in expansion of
macro ‘TEST_ENUM_SMALL_TYPE_NAME’
 TEST_ENUM_SMALL_TYPE_NAME enum_small_random[] =
{T(0),T(1),T(1),T(0),T(0),T(1),T(1),T(1),T(0),T(0),T(0),T(0),T(0),T(1),T(1),T(1),T(1),T(1),T(1),T(0),T(1),T(1),T(0),T(1),T(1),T(0)
};
 ^~~~~~~~~~~~~~~~~~~~~~~~~
...
so it probably doesn't fix the issue completely.


On Thu, Oct 17, 2019 at 10:44 PM Martin Jansa <martin.jansa@gmail.com>
wrote:

> On Sun, Sep 29, 2019 at 05:29:56PM +0800, Chen Qi wrote:
> > Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> > ---
> >  .../0001-avoid-race-condition.patch           | 36 +++++++++++++++++++
> >  .../protobuf/protobuf-c_1.3.2.bb              |  1 +
> >  2 files changed, 37 insertions(+)
> >  create mode 100644
> meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> >
> > diff --git
> a/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> > new file mode 100644
> > index 000000000..4fc7703d8
> > --- /dev/null
> > +++
> b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> > @@ -0,0 +1,36 @@
> > +From 216e31260b618ec73862f9f5336597f391444dac Mon Sep 17 00:00:00 2001
> > +From: Chen Qi <Qi.Chen@windriver.com>
> > +Date: Sun, 29 Sep 2019 17:20:42 +0800
> > +Subject: [PATCH] avoid race condition
> > +
> > +It's possible that the cxx-generate-packed-data.cc is compiled
> > +while the t/test-full.pb.h is being generated. This will result
> > +the following error.
> > +
> > +  DEBUG:     ./t/test-full.pb.h:4:0: error: unterminated #ifndef
> > +  ./t/test-full.pb.h:4:0: error: unterminated #ifndef
> > +
> > +Add a dependency to avoid such problem.
> > +
> > +Upstream-Status: Pending
>
> Thanks for this patch, it seems to indeed fix this issue, I've reported
> a while ago:
>
> https://www.mail-archive.com/openembedded-devel@lists.openembedded.org/msg64363.html
>
> I think the upstream would be interested in this fix as well, see:
>
> https://github.com/protobuf-c/protobuf-c/commit/b0bb56303366e2c072ee38eb5f1f11251b07239c
>
> Cheers,
>
> > +
> > +Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> > +---
> > + Makefile.am | 1 +
> > + 1 file changed, 1 insertion(+)
> > +
> > +diff --git a/Makefile.am b/Makefile.am
> > +index b0cb065..1608ae0 100644
> > +--- a/Makefile.am
> > ++++ b/Makefile.am
> > +@@ -156,6 +156,7 @@ noinst_PROGRAMS += \
> > + t_generated_code2_cxx_generate_packed_data_SOURCES = \
> > +     t/generated-code2/cxx-generate-packed-data.cc \
> > +     t/test-full.pb.cc
> > ++t/generated-code2/cxx-generate-packed-data.cc: t/test-full.pb.h
> > + $(t_generated_code2_cxx_generate_packed_data_OBJECTS): t/test-full.pb.h
> > + t_generated_code2_cxx_generate_packed_data_CXXFLAGS = \
> > +     $(AM_CXXFLAGS) \
> > +--
> > +2.17.1
> > +
> > diff --git a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> > index 6d1ffc3f4..b92f82dec 100644
> > --- a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> > +++ b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> > @@ -15,6 +15,7 @@ DEPENDS = "protobuf-native protobuf"
> >  SRCREV = "1390409f4ee4e26d0635310995b516eb702c3f9e"
> >
> >  SRC_URI = "git://github.com/protobuf-c/protobuf-c.git \
> > +           file://0001-avoid-race-condition.patch \
> >            "
> >
> >  S = "${WORKDIR}/git"
> > --
> > 2.17.1
> >
> > --
> > _______________________________________________
> > Openembedded-devel mailing list
> > Openembedded-devel@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-devel
>
> --
> Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com
>


^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] protobuf-c: fix race condition
  2019-10-17 22:11   ` Martin Jansa
@ 2019-10-18  1:15     ` Sinan Kaya
  2019-10-18  9:07     ` Khem Raj
  1 sibling, 0 replies; 9+ messages in thread
From: Sinan Kaya @ 2019-10-18  1:15 UTC (permalink / raw)
  To: openembedded-devel

On 10/17/2019 3:11 PM, Martin Jansa wrote:
> Hmm I spoke too soon, at least rebuilding protobuf-c-native with this
> change backported to thud still failed once with:

Maybe, there are multiple issues as you mentioned. I'm monitoring our
build pipeline for failures too.

I have always seen this:

+  DEBUG:     ./t/test-full.pb.h:4:0: error: unterminated #ifndef
+  ./t/test-full.pb.h:4:0: error: unterminated #ifndef



^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] protobuf-c: fix race condition
  2019-10-17 22:11   ` Martin Jansa
  2019-10-18  1:15     ` Sinan Kaya
@ 2019-10-18  9:07     ` Khem Raj
  2019-10-18 19:06       ` Martin Jansa
  1 sibling, 1 reply; 9+ messages in thread
From: Khem Raj @ 2019-10-18  9:07 UTC (permalink / raw)
  To: Martin Jansa; +Cc: openembedded-devel

Yes I have seen same build failures on master even after this fix on
machine with more cpus so the fix while seems to address the problem is not
fixing is completely when I get a chance I will look into the logic myself
too but I think we have to delve into it a bit More

On Fri, Oct 18, 2019 at 3:41 AM Martin Jansa <martin.jansa@gmail.com> wrote:

> Hmm I spoke too soon, at least rebuilding protobuf-c-native with this
> change backported to thud still failed once with:
>
> ../git/t/generated-code2/cxx-generate-packed-data.cc:28:17: error: ‘foo’ is
> not a namespace-name
>  using namespace foo;
>                  ^~~
> ../git/t/generated-code2/cxx-generate-packed-data.cc:28:20: error: expected
> namespace-name before ‘;’ token
>  using namespace foo;
>                     ^
> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
> ‘TestEnumSmall’ does not name a type
>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
>                                     ^
> ../git/t/generated-code2/common-test-arrays.h:44:1: note: in expansion of
> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
>  TEST_ENUM_SMALL_TYPE_NAME enum_small_0[] = { TEST_ENUM_SMALL(VALUE) };
>  ^~~~~~~~~~~~~~~~~~~~~~~~~
> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
> ‘TestEnumSmall’ does not name a type
>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
>                                     ^
> ../git/t/generated-code2/common-test-arrays.h:45:1: note: in expansion of
> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
>  TEST_ENUM_SMALL_TYPE_NAME enum_small_1[] = { TEST_ENUM_SMALL(OTHER_VALUE)
> };
>  ^~~~~~~~~~~~~~~~~~~~~~~~~
> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
> ‘TestEnumSmall’ does not name a type
>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
>                                     ^
> ../git/t/generated-code2/common-test-arrays.h:47:1: note: in expansion of
> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
>  TEST_ENUM_SMALL_TYPE_NAME enum_small_random[] =
>
> {T(0),T(1),T(1),T(0),T(0),T(1),T(1),T(1),T(0),T(0),T(0),T(0),T(0),T(1),T(1),T(1),T(1),T(1),T(1),T(0),T(1),T(1),T(0),T(1),T(1),T(0)
> };
>  ^~~~~~~~~~~~~~~~~~~~~~~~~
> ...
> so it probably doesn't fix the issue completely.
>
>
> On Thu, Oct 17, 2019 at 10:44 PM Martin Jansa <martin.jansa@gmail.com>
> wrote:
>
> > On Sun, Sep 29, 2019 at 05:29:56PM +0800, Chen Qi wrote:
> > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> > > ---
> > >  .../0001-avoid-race-condition.patch           | 36 +++++++++++++++++++
> > >  .../protobuf/protobuf-c_1.3.2.bb              |  1 +
> > >  2 files changed, 37 insertions(+)
> > >  create mode 100644
> >
> meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> > >
> > > diff --git
> >
> a/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> >
> b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> > > new file mode 100644
> > > index 000000000..4fc7703d8
> > > --- /dev/null
> > > +++
> >
> b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> > > @@ -0,0 +1,36 @@
> > > +From 216e31260b618ec73862f9f5336597f391444dac Mon Sep 17 00:00:00 2001
> > > +From: Chen Qi <Qi.Chen@windriver.com>
> > > +Date: Sun, 29 Sep 2019 17:20:42 +0800
> > > +Subject: [PATCH] avoid race condition
> > > +
> > > +It's possible that the cxx-generate-packed-data.cc is compiled
> > > +while the t/test-full.pb.h is being generated. This will result
> > > +the following error.
> > > +
> > > +  DEBUG:     ./t/test-full.pb.h:4:0: error: unterminated #ifndef
> > > +  ./t/test-full.pb.h:4:0: error: unterminated #ifndef
> > > +
> > > +Add a dependency to avoid such problem.
> > > +
> > > +Upstream-Status: Pending
> >
> > Thanks for this patch, it seems to indeed fix this issue, I've reported
> > a while ago:
> >
> >
> https://www.mail-archive.com/openembedded-devel@lists.openembedded.org/msg64363.html
> >
> > I think the upstream would be interested in this fix as well, see:
> >
> >
> https://github.com/protobuf-c/protobuf-c/commit/b0bb56303366e2c072ee38eb5f1f11251b07239c
> >
> > Cheers,
> >
> > > +
> > > +Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> > > +---
> > > + Makefile.am | 1 +
> > > + 1 file changed, 1 insertion(+)
> > > +
> > > +diff --git a/Makefile.am b/Makefile.am
> > > +index b0cb065..1608ae0 100644
> > > +--- a/Makefile.am
> > > ++++ b/Makefile.am
> > > +@@ -156,6 +156,7 @@ noinst_PROGRAMS += \
> > > + t_generated_code2_cxx_generate_packed_data_SOURCES = \
> > > +     t/generated-code2/cxx-generate-packed-data.cc \
> > > +     t/test-full.pb.cc
> > > ++t/generated-code2/cxx-generate-packed-data.cc: t/test-full.pb.h
> > > + $(t_generated_code2_cxx_generate_packed_data_OBJECTS):
> t/test-full.pb.h
> > > + t_generated_code2_cxx_generate_packed_data_CXXFLAGS = \
> > > +     $(AM_CXXFLAGS) \
> > > +--
> > > +2.17.1
> > > +
> > > diff --git a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> > b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> > > index 6d1ffc3f4..b92f82dec 100644
> > > --- a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> > > +++ b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> > > @@ -15,6 +15,7 @@ DEPENDS = "protobuf-native protobuf"
> > >  SRCREV = "1390409f4ee4e26d0635310995b516eb702c3f9e"
> > >
> > >  SRC_URI = "git://github.com/protobuf-c/protobuf-c.git \
> > > +           file://0001-avoid-race-condition.patch \
> > >            "
> > >
> > >  S = "${WORKDIR}/git"
> > > --
> > > 2.17.1
> > >
> > > --
> > > _______________________________________________
> > > Openembedded-devel mailing list
> > > Openembedded-devel@lists.openembedded.org
> > > http://lists.openembedded.org/mailman/listinfo/openembedded-devel
> >
> > --
> > Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com
> >
> --
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-devel
>


^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] protobuf-c: fix race condition
  2019-10-18  9:07     ` Khem Raj
@ 2019-10-18 19:06       ` Martin Jansa
  2019-10-22 21:37         ` Martin Jansa
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Jansa @ 2019-10-18 19:06 UTC (permalink / raw)
  To: Khem Raj; +Cc: openembedded-devel

One more data point is that it might be fixed completely with 1.3.2 version
used in zeus/master and failing less often then without this patch in 1.3.1
used in thud/warrior.

I was rebuilding it 300 times in thud/warrior/zeus and got 5-6 failures in
thud/warrior but none in zeus.

I've started another for loop to rebuilt it 1000 time with zeus and if it
works then I'll retest warrior and thud with 1.3.2 upgrade backported to
them - if it works then we can either backport whole upgrade or check the
1.3.1 1.3.2 diff to find out what else we need to backport from there.

On Fri, Oct 18, 2019 at 11:07 AM Khem Raj <raj.khem@gmail.com> wrote:

> Yes I have seen same build failures on master even after this fix on
> machine with more cpus so the fix while seems to address the problem is not
> fixing is completely when I get a chance I will look into the logic myself
> too but I think we have to delve into it a bit More
>
> On Fri, Oct 18, 2019 at 3:41 AM Martin Jansa <martin.jansa@gmail.com>
> wrote:
>
>> Hmm I spoke too soon, at least rebuilding protobuf-c-native with this
>> change backported to thud still failed once with:
>>
>> ../git/t/generated-code2/cxx-generate-packed-data.cc:28:17: error: ‘foo’
>> is
>> not a namespace-name
>>  using namespace foo;
>>                  ^~~
>> ../git/t/generated-code2/cxx-generate-packed-data.cc:28:20: error:
>> expected
>> namespace-name before ‘;’ token
>>  using namespace foo;
>>                     ^
>> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
>> ‘TestEnumSmall’ does not name a type
>>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
>>                                     ^
>> ../git/t/generated-code2/common-test-arrays.h:44:1: note: in expansion of
>> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
>>  TEST_ENUM_SMALL_TYPE_NAME enum_small_0[] = { TEST_ENUM_SMALL(VALUE) };
>>  ^~~~~~~~~~~~~~~~~~~~~~~~~
>> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
>> ‘TestEnumSmall’ does not name a type
>>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
>>                                     ^
>> ../git/t/generated-code2/common-test-arrays.h:45:1: note: in expansion of
>> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
>>  TEST_ENUM_SMALL_TYPE_NAME enum_small_1[] = { TEST_ENUM_SMALL(OTHER_VALUE)
>> };
>>  ^~~~~~~~~~~~~~~~~~~~~~~~~
>> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
>> ‘TestEnumSmall’ does not name a type
>>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
>>                                     ^
>> ../git/t/generated-code2/common-test-arrays.h:47:1: note: in expansion of
>> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
>>  TEST_ENUM_SMALL_TYPE_NAME enum_small_random[] =
>>
>> {T(0),T(1),T(1),T(0),T(0),T(1),T(1),T(1),T(0),T(0),T(0),T(0),T(0),T(1),T(1),T(1),T(1),T(1),T(1),T(0),T(1),T(1),T(0),T(1),T(1),T(0)
>> };
>>  ^~~~~~~~~~~~~~~~~~~~~~~~~
>> ...
>> so it probably doesn't fix the issue completely.
>>
>>
>> On Thu, Oct 17, 2019 at 10:44 PM Martin Jansa <martin.jansa@gmail.com>
>> wrote:
>>
>> > On Sun, Sep 29, 2019 at 05:29:56PM +0800, Chen Qi wrote:
>> > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>> > > ---
>> > >  .../0001-avoid-race-condition.patch           | 36
>> +++++++++++++++++++
>> > >  .../protobuf/protobuf-c_1.3.2.bb              |  1 +
>> > >  2 files changed, 37 insertions(+)
>> > >  create mode 100644
>> >
>> meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
>> > >
>> > > diff --git
>> >
>> a/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
>> >
>> b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
>> > > new file mode 100644
>> > > index 000000000..4fc7703d8
>> > > --- /dev/null
>> > > +++
>> >
>> b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
>> > > @@ -0,0 +1,36 @@
>> > > +From 216e31260b618ec73862f9f5336597f391444dac Mon Sep 17 00:00:00
>> 2001
>> > > +From: Chen Qi <Qi.Chen@windriver.com>
>> > > +Date: Sun, 29 Sep 2019 17:20:42 +0800
>> > > +Subject: [PATCH] avoid race condition
>> > > +
>> > > +It's possible that the cxx-generate-packed-data.cc is compiled
>> > > +while the t/test-full.pb.h is being generated. This will result
>> > > +the following error.
>> > > +
>> > > +  DEBUG:     ./t/test-full.pb.h:4:0: error: unterminated #ifndef
>> > > +  ./t/test-full.pb.h:4:0: error: unterminated #ifndef
>> > > +
>> > > +Add a dependency to avoid such problem.
>> > > +
>> > > +Upstream-Status: Pending
>> >
>> > Thanks for this patch, it seems to indeed fix this issue, I've reported
>> > a while ago:
>> >
>> >
>> https://www.mail-archive.com/openembedded-devel@lists.openembedded.org/msg64363.html
>> >
>> > I think the upstream would be interested in this fix as well, see:
>> >
>> >
>> https://github.com/protobuf-c/protobuf-c/commit/b0bb56303366e2c072ee38eb5f1f11251b07239c
>> >
>> > Cheers,
>> >
>> > > +
>> > > +Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>> > > +---
>> > > + Makefile.am | 1 +
>> > > + 1 file changed, 1 insertion(+)
>> > > +
>> > > +diff --git a/Makefile.am b/Makefile.am
>> > > +index b0cb065..1608ae0 100644
>> > > +--- a/Makefile.am
>> > > ++++ b/Makefile.am
>> > > +@@ -156,6 +156,7 @@ noinst_PROGRAMS += \
>> > > + t_generated_code2_cxx_generate_packed_data_SOURCES = \
>> > > +     t/generated-code2/cxx-generate-packed-data.cc \
>> > > +     t/test-full.pb.cc
>> > > ++t/generated-code2/cxx-generate-packed-data.cc: t/test-full.pb.h
>> > > + $(t_generated_code2_cxx_generate_packed_data_OBJECTS):
>> t/test-full.pb.h
>> > > + t_generated_code2_cxx_generate_packed_data_CXXFLAGS = \
>> > > +     $(AM_CXXFLAGS) \
>> > > +--
>> > > +2.17.1
>> > > +
>> > > diff --git a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
>> > b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
>> > > index 6d1ffc3f4..b92f82dec 100644
>> > > --- a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
>> > > +++ b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
>> > > @@ -15,6 +15,7 @@ DEPENDS = "protobuf-native protobuf"
>> > >  SRCREV = "1390409f4ee4e26d0635310995b516eb702c3f9e"
>> > >
>> > >  SRC_URI = "git://github.com/protobuf-c/protobuf-c.git \
>> > > +           file://0001-avoid-race-condition.patch \
>> > >            "
>> > >
>> > >  S = "${WORKDIR}/git"
>> > > --
>> > > 2.17.1
>> > >
>> > > --
>> > > _______________________________________________
>> > > Openembedded-devel mailing list
>> > > Openembedded-devel@lists.openembedded.org
>> > > http://lists.openembedded.org/mailman/listinfo/openembedded-devel
>> >
>> > --
>> > Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com
>> >
>> --
>> _______________________________________________
>> Openembedded-devel mailing list
>> Openembedded-devel@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-devel
>>
>


^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] protobuf-c: fix race condition
  2019-10-18 19:06       ` Martin Jansa
@ 2019-10-22 21:37         ` Martin Jansa
  2019-10-23 15:50           ` akuster808
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Jansa @ 2019-10-22 21:37 UTC (permalink / raw)
  To: Khem Raj; +Cc: openembedded-devel

[-- Attachment #1: Type: text/plain, Size: 8650 bytes --]

On Fri, Oct 18, 2019 at 09:06:54PM +0200, Martin Jansa wrote:
> One more data point is that it might be fixed completely with 1.3.2 version
> used in zeus/master and failing less often then without this patch in 1.3.1
> used in thud/warrior.
> 
> I was rebuilding it 300 times in thud/warrior/zeus and got 5-6 failures in
> thud/warrior but none in zeus.
> 
> I've started another for loop to rebuilt it 1000 time with zeus and if it
> works then I'll retest warrior and thud with 1.3.2 upgrade backported to
> them - if it works then we can either backport whole upgrade or check the
> 1.3.1 1.3.2 diff to find out what else we need to backport from there.

1000 times in zeus = 0 failures
100 times in thud with 1.3.2 backported ~ 10 failures
1000 times in thud with 1.3.2 as well as protobuf-native 3.9.2 = 0 failures

The protobuf-c upgrade isn't need to be backported to thud, there aren't
many changes:
https://github.com/protobuf-c/protobuf-c/compare/269771b4b45d3aba04e59569f53600003db8d9ff...1390409f4ee4e26d0635310995b516eb702c3f9e

protobuf on the other hand was upgraded from 3.6.1 to 3.9.2 in zeus and
the diff is big (1028 commits):
https://github.com/protocolbuffers/protobuf/compare/48cb18e5c419ddd23d9badcfe4e9df7bde1979b2...52b2447247f535663ac1c292e088b4b27d2910ef

quick grep in git log didn't reveal what might be causing the race
condition, upgrading whole protobuf in warrior and thud isn't an option
as well.

Anyone interested in digging a bit deeper?

I don't want to block this change, because it seems to completely fix it
for zeus/master and partially improve it also for warrior and thud.

We don't use protobuf much, so for our builds I think I'll just backport
whole protobuf upgrade to thud once protobuf-c fix is in thud.

Regards,

> On Fri, Oct 18, 2019 at 11:07 AM Khem Raj <raj.khem@gmail.com> wrote:
> 
> > Yes I have seen same build failures on master even after this fix on
> > machine with more cpus so the fix while seems to address the problem is not
> > fixing is completely when I get a chance I will look into the logic myself
> > too but I think we have to delve into it a bit More
> >
> > On Fri, Oct 18, 2019 at 3:41 AM Martin Jansa <martin.jansa@gmail.com>
> > wrote:
> >
> >> Hmm I spoke too soon, at least rebuilding protobuf-c-native with this
> >> change backported to thud still failed once with:
> >>
> >> ../git/t/generated-code2/cxx-generate-packed-data.cc:28:17: error: ‘foo’
> >> is
> >> not a namespace-name
> >>  using namespace foo;
> >>                  ^~~
> >> ../git/t/generated-code2/cxx-generate-packed-data.cc:28:20: error:
> >> expected
> >> namespace-name before ‘;’ token
> >>  using namespace foo;
> >>                     ^
> >> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
> >> ‘TestEnumSmall’ does not name a type
> >>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
> >>                                     ^
> >> ../git/t/generated-code2/common-test-arrays.h:44:1: note: in expansion of
> >> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
> >>  TEST_ENUM_SMALL_TYPE_NAME enum_small_0[] = { TEST_ENUM_SMALL(VALUE) };
> >>  ^~~~~~~~~~~~~~~~~~~~~~~~~
> >> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
> >> ‘TestEnumSmall’ does not name a type
> >>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
> >>                                     ^
> >> ../git/t/generated-code2/common-test-arrays.h:45:1: note: in expansion of
> >> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
> >>  TEST_ENUM_SMALL_TYPE_NAME enum_small_1[] = { TEST_ENUM_SMALL(OTHER_VALUE)
> >> };
> >>  ^~~~~~~~~~~~~~~~~~~~~~~~~
> >> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
> >> ‘TestEnumSmall’ does not name a type
> >>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
> >>                                     ^
> >> ../git/t/generated-code2/common-test-arrays.h:47:1: note: in expansion of
> >> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
> >>  TEST_ENUM_SMALL_TYPE_NAME enum_small_random[] =
> >>
> >> {T(0),T(1),T(1),T(0),T(0),T(1),T(1),T(1),T(0),T(0),T(0),T(0),T(0),T(1),T(1),T(1),T(1),T(1),T(1),T(0),T(1),T(1),T(0),T(1),T(1),T(0)
> >> };
> >>  ^~~~~~~~~~~~~~~~~~~~~~~~~
> >> ...
> >> so it probably doesn't fix the issue completely.
> >>
> >>
> >> On Thu, Oct 17, 2019 at 10:44 PM Martin Jansa <martin.jansa@gmail.com>
> >> wrote:
> >>
> >> > On Sun, Sep 29, 2019 at 05:29:56PM +0800, Chen Qi wrote:
> >> > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> >> > > ---
> >> > >  .../0001-avoid-race-condition.patch           | 36
> >> +++++++++++++++++++
> >> > >  .../protobuf/protobuf-c_1.3.2.bb              |  1 +
> >> > >  2 files changed, 37 insertions(+)
> >> > >  create mode 100644
> >> >
> >> meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> >> > >
> >> > > diff --git
> >> >
> >> a/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> >> >
> >> b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> >> > > new file mode 100644
> >> > > index 000000000..4fc7703d8
> >> > > --- /dev/null
> >> > > +++
> >> >
> >> b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> >> > > @@ -0,0 +1,36 @@
> >> > > +From 216e31260b618ec73862f9f5336597f391444dac Mon Sep 17 00:00:00
> >> 2001
> >> > > +From: Chen Qi <Qi.Chen@windriver.com>
> >> > > +Date: Sun, 29 Sep 2019 17:20:42 +0800
> >> > > +Subject: [PATCH] avoid race condition
> >> > > +
> >> > > +It's possible that the cxx-generate-packed-data.cc is compiled
> >> > > +while the t/test-full.pb.h is being generated. This will result
> >> > > +the following error.
> >> > > +
> >> > > +  DEBUG:     ./t/test-full.pb.h:4:0: error: unterminated #ifndef
> >> > > +  ./t/test-full.pb.h:4:0: error: unterminated #ifndef
> >> > > +
> >> > > +Add a dependency to avoid such problem.
> >> > > +
> >> > > +Upstream-Status: Pending
> >> >
> >> > Thanks for this patch, it seems to indeed fix this issue, I've reported
> >> > a while ago:
> >> >
> >> >
> >> https://www.mail-archive.com/openembedded-devel@lists.openembedded.org/msg64363.html
> >> >
> >> > I think the upstream would be interested in this fix as well, see:
> >> >
> >> >
> >> https://github.com/protobuf-c/protobuf-c/commit/b0bb56303366e2c072ee38eb5f1f11251b07239c
> >> >
> >> > Cheers,
> >> >
> >> > > +
> >> > > +Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> >> > > +---
> >> > > + Makefile.am | 1 +
> >> > > + 1 file changed, 1 insertion(+)
> >> > > +
> >> > > +diff --git a/Makefile.am b/Makefile.am
> >> > > +index b0cb065..1608ae0 100644
> >> > > +--- a/Makefile.am
> >> > > ++++ b/Makefile.am
> >> > > +@@ -156,6 +156,7 @@ noinst_PROGRAMS += \
> >> > > + t_generated_code2_cxx_generate_packed_data_SOURCES = \
> >> > > +     t/generated-code2/cxx-generate-packed-data.cc \
> >> > > +     t/test-full.pb.cc
> >> > > ++t/generated-code2/cxx-generate-packed-data.cc: t/test-full.pb.h
> >> > > + $(t_generated_code2_cxx_generate_packed_data_OBJECTS):
> >> t/test-full.pb.h
> >> > > + t_generated_code2_cxx_generate_packed_data_CXXFLAGS = \
> >> > > +     $(AM_CXXFLAGS) \
> >> > > +--
> >> > > +2.17.1
> >> > > +
> >> > > diff --git a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> >> > b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> >> > > index 6d1ffc3f4..b92f82dec 100644
> >> > > --- a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> >> > > +++ b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> >> > > @@ -15,6 +15,7 @@ DEPENDS = "protobuf-native protobuf"
> >> > >  SRCREV = "1390409f4ee4e26d0635310995b516eb702c3f9e"
> >> > >
> >> > >  SRC_URI = "git://github.com/protobuf-c/protobuf-c.git \
> >> > > +           file://0001-avoid-race-condition.patch \
> >> > >            "
> >> > >
> >> > >  S = "${WORKDIR}/git"
> >> > > --
> >> > > 2.17.1
> >> > >
> >> > > --
> >> > > _______________________________________________
> >> > > Openembedded-devel mailing list
> >> > > Openembedded-devel@lists.openembedded.org
> >> > > http://lists.openembedded.org/mailman/listinfo/openembedded-devel
> >> >
> >> > --
> >> > Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com
> >> >
> >> --
> >> _______________________________________________
> >> Openembedded-devel mailing list
> >> Openembedded-devel@lists.openembedded.org
> >> http://lists.openembedded.org/mailman/listinfo/openembedded-devel
> >>
> >

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 201 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] protobuf-c: fix race condition
  2019-10-22 21:37         ` Martin Jansa
@ 2019-10-23 15:50           ` akuster808
  2019-10-23 16:23             ` Martin Jansa
  0 siblings, 1 reply; 9+ messages in thread
From: akuster808 @ 2019-10-23 15:50 UTC (permalink / raw)
  To: Martin Jansa, Khem Raj; +Cc: openembedded-devel



On 10/22/19 2:37 PM, Martin Jansa wrote:
> On Fri, Oct 18, 2019 at 09:06:54PM +0200, Martin Jansa wrote:
>> One more data point is that it might be fixed completely with 1.3.2 version
>> used in zeus/master and failing less often then without this patch in 1.3.1
>> used in thud/warrior.
>>
>> I was rebuilding it 300 times in thud/warrior/zeus and got 5-6 failures in
>> thud/warrior but none in zeus.
>>
>> I've started another for loop to rebuilt it 1000 time with zeus and if it
>> works then I'll retest warrior and thud with 1.3.2 upgrade backported to
>> them - if it works then we can either backport whole upgrade or check the
>> 1.3.1 1.3.2 diff to find out what else we need to backport from there.
> 1000 times in zeus = 0 failures
> 100 times in thud with 1.3.2 backported ~ 10 failures
> 1000 times in thud with 1.3.2 as well as protobuf-native 3.9.2 = 0 failures
>
> The protobuf-c upgrade isn't need to be backported to thud, there aren't
> many changes:
> https://github.com/protobuf-c/protobuf-c/compare/269771b4b45d3aba04e59569f53600003db8d9ff...1390409f4ee4e26d0635310995b516eb702c3f9e
>
> protobuf on the other hand was upgraded from 3.6.1 to 3.9.2 in zeus and
> the diff is big (1028 commits):
> https://github.com/protocolbuffers/protobuf/compare/48cb18e5c419ddd23d9badcfe4e9df7bde1979b2...52b2447247f535663ac1c292e088b4b27d2910ef
>
> quick grep in git log didn't reveal what might be causing the race
> condition, upgrading whole protobuf in warrior and thud isn't an option
> as well.
>
> Anyone interested in digging a bit deeper?
>
> I don't want to block this change, because it seems to completely fix it
> for zeus/master and partially improve it also for warrior and thud.
>
> We don't use protobuf much, so for our builds I think I'll just backport
> whole protobuf upgrade to thud once protobuf-c fix is in thud.

So is https://patchwork.openembedded.org/patch/165905/ still a go for
warrior?

- armin
> Regards,
>
>> On Fri, Oct 18, 2019 at 11:07 AM Khem Raj <raj.khem@gmail.com> wrote:
>>
>>> Yes I have seen same build failures on master even after this fix on
>>> machine with more cpus so the fix while seems to address the problem is not
>>> fixing is completely when I get a chance I will look into the logic myself
>>> too but I think we have to delve into it a bit More
>>>
>>> On Fri, Oct 18, 2019 at 3:41 AM Martin Jansa <martin.jansa@gmail.com>
>>> wrote:
>>>
>>>> Hmm I spoke too soon, at least rebuilding protobuf-c-native with this
>>>> change backported to thud still failed once with:
>>>>
>>>> ../git/t/generated-code2/cxx-generate-packed-data.cc:28:17: error: ‘foo’
>>>> is
>>>> not a namespace-name
>>>>  using namespace foo;
>>>>                  ^~~
>>>> ../git/t/generated-code2/cxx-generate-packed-data.cc:28:20: error:
>>>> expected
>>>> namespace-name before ‘;’ token
>>>>  using namespace foo;
>>>>                     ^
>>>> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
>>>> ‘TestEnumSmall’ does not name a type
>>>>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
>>>>                                     ^
>>>> ../git/t/generated-code2/common-test-arrays.h:44:1: note: in expansion of
>>>> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
>>>>  TEST_ENUM_SMALL_TYPE_NAME enum_small_0[] = { TEST_ENUM_SMALL(VALUE) };
>>>>  ^~~~~~~~~~~~~~~~~~~~~~~~~
>>>> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
>>>> ‘TestEnumSmall’ does not name a type
>>>>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
>>>>                                     ^
>>>> ../git/t/generated-code2/common-test-arrays.h:45:1: note: in expansion of
>>>> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
>>>>  TEST_ENUM_SMALL_TYPE_NAME enum_small_1[] = { TEST_ENUM_SMALL(OTHER_VALUE)
>>>> };
>>>>  ^~~~~~~~~~~~~~~~~~~~~~~~~
>>>> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
>>>> ‘TestEnumSmall’ does not name a type
>>>>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
>>>>                                     ^
>>>> ../git/t/generated-code2/common-test-arrays.h:47:1: note: in expansion of
>>>> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
>>>>  TEST_ENUM_SMALL_TYPE_NAME enum_small_random[] =
>>>>
>>>> {T(0),T(1),T(1),T(0),T(0),T(1),T(1),T(1),T(0),T(0),T(0),T(0),T(0),T(1),T(1),T(1),T(1),T(1),T(1),T(0),T(1),T(1),T(0),T(1),T(1),T(0)
>>>> };
>>>>  ^~~~~~~~~~~~~~~~~~~~~~~~~
>>>> ...
>>>> so it probably doesn't fix the issue completely.
>>>>
>>>>
>>>> On Thu, Oct 17, 2019 at 10:44 PM Martin Jansa <martin.jansa@gmail.com>
>>>> wrote:
>>>>
>>>>> On Sun, Sep 29, 2019 at 05:29:56PM +0800, Chen Qi wrote:
>>>>>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>>>>>> ---
>>>>>>  .../0001-avoid-race-condition.patch           | 36
>>>> +++++++++++++++++++
>>>>>>  .../protobuf/protobuf-c_1.3.2.bb              |  1 +
>>>>>>  2 files changed, 37 insertions(+)
>>>>>>  create mode 100644
>>>> meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
>>>>>> diff --git
>>>> a/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
>>>> b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
>>>>>> new file mode 100644
>>>>>> index 000000000..4fc7703d8
>>>>>> --- /dev/null
>>>>>> +++
>>>> b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
>>>>>> @@ -0,0 +1,36 @@
>>>>>> +From 216e31260b618ec73862f9f5336597f391444dac Mon Sep 17 00:00:00
>>>> 2001
>>>>>> +From: Chen Qi <Qi.Chen@windriver.com>
>>>>>> +Date: Sun, 29 Sep 2019 17:20:42 +0800
>>>>>> +Subject: [PATCH] avoid race condition
>>>>>> +
>>>>>> +It's possible that the cxx-generate-packed-data.cc is compiled
>>>>>> +while the t/test-full.pb.h is being generated. This will result
>>>>>> +the following error.
>>>>>> +
>>>>>> +  DEBUG:     ./t/test-full.pb.h:4:0: error: unterminated #ifndef
>>>>>> +  ./t/test-full.pb.h:4:0: error: unterminated #ifndef
>>>>>> +
>>>>>> +Add a dependency to avoid such problem.
>>>>>> +
>>>>>> +Upstream-Status: Pending
>>>>> Thanks for this patch, it seems to indeed fix this issue, I've reported
>>>>> a while ago:
>>>>>
>>>>>
>>>> https://www.mail-archive.com/openembedded-devel@lists.openembedded.org/msg64363.html
>>>>> I think the upstream would be interested in this fix as well, see:
>>>>>
>>>>>
>>>> https://github.com/protobuf-c/protobuf-c/commit/b0bb56303366e2c072ee38eb5f1f11251b07239c
>>>>> Cheers,
>>>>>
>>>>>> +
>>>>>> +Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>>>>>> +---
>>>>>> + Makefile.am | 1 +
>>>>>> + 1 file changed, 1 insertion(+)
>>>>>> +
>>>>>> +diff --git a/Makefile.am b/Makefile.am
>>>>>> +index b0cb065..1608ae0 100644
>>>>>> +--- a/Makefile.am
>>>>>> ++++ b/Makefile.am
>>>>>> +@@ -156,6 +156,7 @@ noinst_PROGRAMS += \
>>>>>> + t_generated_code2_cxx_generate_packed_data_SOURCES = \
>>>>>> +     t/generated-code2/cxx-generate-packed-data.cc \
>>>>>> +     t/test-full.pb.cc
>>>>>> ++t/generated-code2/cxx-generate-packed-data.cc: t/test-full.pb.h
>>>>>> + $(t_generated_code2_cxx_generate_packed_data_OBJECTS):
>>>> t/test-full.pb.h
>>>>>> + t_generated_code2_cxx_generate_packed_data_CXXFLAGS = \
>>>>>> +     $(AM_CXXFLAGS) \
>>>>>> +--
>>>>>> +2.17.1
>>>>>> +
>>>>>> diff --git a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
>>>>> b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
>>>>>> index 6d1ffc3f4..b92f82dec 100644
>>>>>> --- a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
>>>>>> +++ b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
>>>>>> @@ -15,6 +15,7 @@ DEPENDS = "protobuf-native protobuf"
>>>>>>  SRCREV = "1390409f4ee4e26d0635310995b516eb702c3f9e"
>>>>>>
>>>>>>  SRC_URI = "git://github.com/protobuf-c/protobuf-c.git \
>>>>>> +           file://0001-avoid-race-condition.patch \
>>>>>>            "
>>>>>>
>>>>>>  S = "${WORKDIR}/git"
>>>>>> --
>>>>>> 2.17.1
>>>>>>
>>>>>> --
>>>>>> _______________________________________________
>>>>>> Openembedded-devel mailing list
>>>>>> Openembedded-devel@lists.openembedded.org
>>>>>> http://lists.openembedded.org/mailman/listinfo/openembedded-devel
>>>>> --
>>>>> Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com
>>>>>
>>>> --
>>>> _______________________________________________
>>>> Openembedded-devel mailing list
>>>> Openembedded-devel@lists.openembedded.org
>>>> http://lists.openembedded.org/mailman/listinfo/openembedded-devel
>>>>
>



^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] protobuf-c: fix race condition
  2019-10-23 15:50           ` akuster808
@ 2019-10-23 16:23             ` Martin Jansa
  0 siblings, 0 replies; 9+ messages in thread
From: Martin Jansa @ 2019-10-23 16:23 UTC (permalink / raw)
  To: akuster808; +Cc: openembedded-devel

[-- Attachment #1: Type: text/plain, Size: 9533 bytes --]

On Wed, Oct 23, 2019 at 08:50:10AM -0700, akuster808 wrote:
> 
> 
> On 10/22/19 2:37 PM, Martin Jansa wrote:
> > On Fri, Oct 18, 2019 at 09:06:54PM +0200, Martin Jansa wrote:
> >> One more data point is that it might be fixed completely with 1.3.2 version
> >> used in zeus/master and failing less often then without this patch in 1.3.1
> >> used in thud/warrior.
> >>
> >> I was rebuilding it 300 times in thud/warrior/zeus and got 5-6 failures in
> >> thud/warrior but none in zeus.
> >>
> >> I've started another for loop to rebuilt it 1000 time with zeus and if it
> >> works then I'll retest warrior and thud with 1.3.2 upgrade backported to
> >> them - if it works then we can either backport whole upgrade or check the
> >> 1.3.1 1.3.2 diff to find out what else we need to backport from there.
> > 1000 times in zeus = 0 failures
> > 100 times in thud with 1.3.2 backported ~ 10 failures
> > 1000 times in thud with 1.3.2 as well as protobuf-native 3.9.2 = 0 failures
> >
> > The protobuf-c upgrade isn't need to be backported to thud, there aren't
> > many changes:
> > https://github.com/protobuf-c/protobuf-c/compare/269771b4b45d3aba04e59569f53600003db8d9ff...1390409f4ee4e26d0635310995b516eb702c3f9e
> >
> > protobuf on the other hand was upgraded from 3.6.1 to 3.9.2 in zeus and
> > the diff is big (1028 commits):
> > https://github.com/protocolbuffers/protobuf/compare/48cb18e5c419ddd23d9badcfe4e9df7bde1979b2...52b2447247f535663ac1c292e088b4b27d2910ef
> >
> > quick grep in git log didn't reveal what might be causing the race
> > condition, upgrading whole protobuf in warrior and thud isn't an option
> > as well.
> >
> > Anyone interested in digging a bit deeper?
> >
> > I don't want to block this change, because it seems to completely fix it
> > for zeus/master and partially improve it also for warrior and thud.
> >
> > We don't use protobuf much, so for our builds I think I'll just backport
> > whole protobuf upgrade to thud once protobuf-c fix is in thud.
> 
> So is https://patchwork.openembedded.org/patch/165905/ still a go for
> warrior?

I think it is and for thud as well. Once there is a fix for protobuf as
well, it should be in separate follow-up change anyway.

with protobuf-c fix included I got 28 failures in 1000 rebuilds
without protobuf-c fix I got 5 failures in 308 rebuilds (still running)

so the rate seems similar if not the same (but there might be other
aspects like load on the build machine from other builds), before I was
seeing lower frequency of failures with this included.

Cheers,

> >> On Fri, Oct 18, 2019 at 11:07 AM Khem Raj <raj.khem@gmail.com> wrote:
> >>
> >>> Yes I have seen same build failures on master even after this fix on
> >>> machine with more cpus so the fix while seems to address the problem is not
> >>> fixing is completely when I get a chance I will look into the logic myself
> >>> too but I think we have to delve into it a bit More
> >>>
> >>> On Fri, Oct 18, 2019 at 3:41 AM Martin Jansa <martin.jansa@gmail.com>
> >>> wrote:
> >>>
> >>>> Hmm I spoke too soon, at least rebuilding protobuf-c-native with this
> >>>> change backported to thud still failed once with:
> >>>>
> >>>> ../git/t/generated-code2/cxx-generate-packed-data.cc:28:17: error: ‘foo’
> >>>> is
> >>>> not a namespace-name
> >>>>  using namespace foo;
> >>>>                  ^~~
> >>>> ../git/t/generated-code2/cxx-generate-packed-data.cc:28:20: error:
> >>>> expected
> >>>> namespace-name before ‘;’ token
> >>>>  using namespace foo;
> >>>>                     ^
> >>>> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
> >>>> ‘TestEnumSmall’ does not name a type
> >>>>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
> >>>>                                     ^
> >>>> ../git/t/generated-code2/common-test-arrays.h:44:1: note: in expansion of
> >>>> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
> >>>>  TEST_ENUM_SMALL_TYPE_NAME enum_small_0[] = { TEST_ENUM_SMALL(VALUE) };
> >>>>  ^~~~~~~~~~~~~~~~~~~~~~~~~
> >>>> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
> >>>> ‘TestEnumSmall’ does not name a type
> >>>>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
> >>>>                                     ^
> >>>> ../git/t/generated-code2/common-test-arrays.h:45:1: note: in expansion of
> >>>> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
> >>>>  TEST_ENUM_SMALL_TYPE_NAME enum_small_1[] = { TEST_ENUM_SMALL(OTHER_VALUE)
> >>>> };
> >>>>  ^~~~~~~~~~~~~~~~~~~~~~~~~
> >>>> ../git/t/generated-code2/cxx-generate-packed-data.cc:31:36: error:
> >>>> ‘TestEnumSmall’ does not name a type
> >>>>  #define TEST_ENUM_SMALL_TYPE_NAME  TestEnumSmall
> >>>>                                     ^
> >>>> ../git/t/generated-code2/common-test-arrays.h:47:1: note: in expansion of
> >>>> macro ‘TEST_ENUM_SMALL_TYPE_NAME’
> >>>>  TEST_ENUM_SMALL_TYPE_NAME enum_small_random[] =
> >>>>
> >>>> {T(0),T(1),T(1),T(0),T(0),T(1),T(1),T(1),T(0),T(0),T(0),T(0),T(0),T(1),T(1),T(1),T(1),T(1),T(1),T(0),T(1),T(1),T(0),T(1),T(1),T(0)
> >>>> };
> >>>>  ^~~~~~~~~~~~~~~~~~~~~~~~~
> >>>> ...
> >>>> so it probably doesn't fix the issue completely.
> >>>>
> >>>>
> >>>> On Thu, Oct 17, 2019 at 10:44 PM Martin Jansa <martin.jansa@gmail.com>
> >>>> wrote:
> >>>>
> >>>>> On Sun, Sep 29, 2019 at 05:29:56PM +0800, Chen Qi wrote:
> >>>>>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> >>>>>> ---
> >>>>>>  .../0001-avoid-race-condition.patch           | 36
> >>>> +++++++++++++++++++
> >>>>>>  .../protobuf/protobuf-c_1.3.2.bb              |  1 +
> >>>>>>  2 files changed, 37 insertions(+)
> >>>>>>  create mode 100644
> >>>> meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> >>>>>> diff --git
> >>>> a/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> >>>> b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> >>>>>> new file mode 100644
> >>>>>> index 000000000..4fc7703d8
> >>>>>> --- /dev/null
> >>>>>> +++
> >>>> b/meta-oe/recipes-devtools/protobuf/protobuf-c/0001-avoid-race-condition.patch
> >>>>>> @@ -0,0 +1,36 @@
> >>>>>> +From 216e31260b618ec73862f9f5336597f391444dac Mon Sep 17 00:00:00
> >>>> 2001
> >>>>>> +From: Chen Qi <Qi.Chen@windriver.com>
> >>>>>> +Date: Sun, 29 Sep 2019 17:20:42 +0800
> >>>>>> +Subject: [PATCH] avoid race condition
> >>>>>> +
> >>>>>> +It's possible that the cxx-generate-packed-data.cc is compiled
> >>>>>> +while the t/test-full.pb.h is being generated. This will result
> >>>>>> +the following error.
> >>>>>> +
> >>>>>> +  DEBUG:     ./t/test-full.pb.h:4:0: error: unterminated #ifndef
> >>>>>> +  ./t/test-full.pb.h:4:0: error: unterminated #ifndef
> >>>>>> +
> >>>>>> +Add a dependency to avoid such problem.
> >>>>>> +
> >>>>>> +Upstream-Status: Pending
> >>>>> Thanks for this patch, it seems to indeed fix this issue, I've reported
> >>>>> a while ago:
> >>>>>
> >>>>>
> >>>> https://www.mail-archive.com/openembedded-devel@lists.openembedded.org/msg64363.html
> >>>>> I think the upstream would be interested in this fix as well, see:
> >>>>>
> >>>>>
> >>>> https://github.com/protobuf-c/protobuf-c/commit/b0bb56303366e2c072ee38eb5f1f11251b07239c
> >>>>> Cheers,
> >>>>>
> >>>>>> +
> >>>>>> +Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> >>>>>> +---
> >>>>>> + Makefile.am | 1 +
> >>>>>> + 1 file changed, 1 insertion(+)
> >>>>>> +
> >>>>>> +diff --git a/Makefile.am b/Makefile.am
> >>>>>> +index b0cb065..1608ae0 100644
> >>>>>> +--- a/Makefile.am
> >>>>>> ++++ b/Makefile.am
> >>>>>> +@@ -156,6 +156,7 @@ noinst_PROGRAMS += \
> >>>>>> + t_generated_code2_cxx_generate_packed_data_SOURCES = \
> >>>>>> +     t/generated-code2/cxx-generate-packed-data.cc \
> >>>>>> +     t/test-full.pb.cc
> >>>>>> ++t/generated-code2/cxx-generate-packed-data.cc: t/test-full.pb.h
> >>>>>> + $(t_generated_code2_cxx_generate_packed_data_OBJECTS):
> >>>> t/test-full.pb.h
> >>>>>> + t_generated_code2_cxx_generate_packed_data_CXXFLAGS = \
> >>>>>> +     $(AM_CXXFLAGS) \
> >>>>>> +--
> >>>>>> +2.17.1
> >>>>>> +
> >>>>>> diff --git a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> >>>>> b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> >>>>>> index 6d1ffc3f4..b92f82dec 100644
> >>>>>> --- a/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> >>>>>> +++ b/meta-oe/recipes-devtools/protobuf/protobuf-c_1.3.2.bb
> >>>>>> @@ -15,6 +15,7 @@ DEPENDS = "protobuf-native protobuf"
> >>>>>>  SRCREV = "1390409f4ee4e26d0635310995b516eb702c3f9e"
> >>>>>>
> >>>>>>  SRC_URI = "git://github.com/protobuf-c/protobuf-c.git \
> >>>>>> +           file://0001-avoid-race-condition.patch \
> >>>>>>            "
> >>>>>>
> >>>>>>  S = "${WORKDIR}/git"
> >>>>>> --
> >>>>>> 2.17.1
> >>>>>>
> >>>>>> --
> >>>>>> _______________________________________________
> >>>>>> Openembedded-devel mailing list
> >>>>>> Openembedded-devel@lists.openembedded.org
> >>>>>> http://lists.openembedded.org/mailman/listinfo/openembedded-devel
> >>>>> --
> >>>>> Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com
> >>>>>
> >>>> --
> >>>> _______________________________________________
> >>>> Openembedded-devel mailing list
> >>>> Openembedded-devel@lists.openembedded.org
> >>>> http://lists.openembedded.org/mailman/listinfo/openembedded-devel
> >>>>
> >
> 

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 201 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-10-23 16:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-29  9:29 [meta-oe][PATCH] protobuf-c: fix race condition Chen Qi
2019-10-17 20:44 ` Martin Jansa
2019-10-17 22:11   ` Martin Jansa
2019-10-18  1:15     ` Sinan Kaya
2019-10-18  9:07     ` Khem Raj
2019-10-18 19:06       ` Martin Jansa
2019-10-22 21:37         ` Martin Jansa
2019-10-23 15:50           ` akuster808
2019-10-23 16:23             ` Martin Jansa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.