[bitbake-devel][PATCH] hashserv: client: Fix AF_UNIX path length limits

[bitbake-devel][PATCH] hashserv: client: Fix AF_UNIX path length limits

[Joshua Watt]

Restores a fix for unix domain socket path length limits when using the
synchronous hash equivalence client that was accidentally removed when
the async client was added.

Unfortunately, it's much more difficult to fix the same problem when
using the async client directly due to the interaction of chdir() and
async code, but this will at least restore the old behavior in the
synchronous case.

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 bitbake/lib/hashserv/client.py | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/bitbake/lib/hashserv/client.py b/bitbake/lib/hashserv/client.py
index ae5875d1b3..7bbf0865d5 100644
--- a/bitbake/lib/hashserv/client.py
+++ b/bitbake/lib/hashserv/client.py
@@ -40,7 +40,7 @@ class AsyncClient(object):
 
         self._connect_sock = connect_sock
 
-    async def _connect(self):
+    async def connect(self):
         if self.reader is None or self.writer is None:
             (self.reader, self.writer) = await self._connect_sock()
 
@@ -62,7 +62,7 @@ class AsyncClient(object):
         count = 0
         while True:
             try:
-                await self._connect()
+                await self.connect()
                 return await proc()
             except (
                 OSError,
@@ -190,7 +190,6 @@ class Client(object):
 
         for call in (
             "connect_tcp",
-            "connect_unix",
             "close",
             "get_unihash",
             "report_unihash",
@@ -209,6 +208,16 @@ class Client(object):
 
         return wrapper
 
+    def connect_unix(self, path):
+        # AF_UNIX has path length issues so chdir here to workaround
+        cwd = os.getcwd()
+        try:
+            os.chdir(os.path.dirname(path))
+            self.loop.run_until_complete(self.client.connect_unix(path))
+            self.loop.run_until_complete(self.client.connect())
+        finally:
+            os.chdir(cwd)
+
     @property
     def max_chunk(self):
         return self.client.max_chunk
-- 
2.29.2

Re: [bitbake-devel][PATCH] hashserv: client: Fix AF_UNIX path length limits

[Nicolas Dechesne]

hi Joshua,


On Wed, Dec 2, 2020 at 8:58 PM Joshua Watt <JPEWhacker@gmail.com> wrote:
>
> Restores a fix for unix domain socket path length limits when using the
> synchronous hash equivalence client that was accidentally removed when
> the async client was added.
>
> Unfortunately, it's much more difficult to fix the same problem when
> using the async client directly due to the interaction of chdir() and
> async code, but this will at least restore the old behavior in the
> synchronous case.
>
> Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>

the meta-qcom CI started failing.. and I have the feeling this is
because of this change. I am now seeing many errors, and this:
00:02:31.320 OSError: AF_UNIX path too long

Here is the link to the full Jenkins log:
https://ci.linaro.org/job/lt-qcom-openembedded-meta-qcom-master-premerge/136/MACHINE=dragonboard-410c,TCLIBC=glibc,label=docker-buster-amd64/console

This is a test build we do for each Github PR. This build is using
this commit from Poky/master:
982688103619 (build-appliance-image: Update to master head revision)

We tested another PR 2 days ago, and it went fine:
https://ci.linaro.org/job/lt-qcom-openembedded-meta-qcom-master-premerge/134/MACHINE=dragonboard-410c,TCLIBC=glibc,label=docker-buster-amd64/consoleFull
This build was based on this commit:
982688103619 (build-appliance-image: Update to master head revision)

Looking at the error log, and this commit, it looks related to me..
though I am not sure I understand the details ;)

> ---
>  bitbake/lib/hashserv/client.py | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/bitbake/lib/hashserv/client.py b/bitbake/lib/hashserv/client.py
> index ae5875d1b3..7bbf0865d5 100644
> --- a/bitbake/lib/hashserv/client.py
> +++ b/bitbake/lib/hashserv/client.py
> @@ -40,7 +40,7 @@ class AsyncClient(object):
>
>          self._connect_sock = connect_sock
>
> -    async def _connect(self):
> +    async def connect(self):
>          if self.reader is None or self.writer is None:
>              (self.reader, self.writer) = await self._connect_sock()
>
> @@ -62,7 +62,7 @@ class AsyncClient(object):
>          count = 0
>          while True:
>              try:
> -                await self._connect()
> +                await self.connect()
>                  return await proc()
>              except (
>                  OSError,
> @@ -190,7 +190,6 @@ class Client(object):
>
>          for call in (
>              "connect_tcp",
> -            "connect_unix",
>              "close",
>              "get_unihash",
>              "report_unihash",
> @@ -209,6 +208,16 @@ class Client(object):
>
>          return wrapper
>
> +    def connect_unix(self, path):
> +        # AF_UNIX has path length issues so chdir here to workaround
> +        cwd = os.getcwd()
> +        try:
> +            os.chdir(os.path.dirname(path))
> +            self.loop.run_until_complete(self.client.connect_unix(path))
> +            self.loop.run_until_complete(self.client.connect())
> +        finally:
> +            os.chdir(cwd)
> +
>      @property
>      def max_chunk(self):
>          return self.client.max_chunk
> --
> 2.29.2
>
>
> 
>

Re: [bitbake-devel][PATCH] hashserv: client: Fix AF_UNIX path length limits

[Joshua Watt]

On Wed, Dec 9, 2020 at 3:04 PM Nicolas Dechesne
<nicolas.dechesne@linaro.org> wrote:
>
> hi Joshua,
>
>
> On Wed, Dec 2, 2020 at 8:58 PM Joshua Watt <JPEWhacker@gmail.com> wrote:
> >
> > Restores a fix for unix domain socket path length limits when using the
> > synchronous hash equivalence client that was accidentally removed when
> > the async client was added.
> >
> > Unfortunately, it's much more difficult to fix the same problem when
> > using the async client directly due to the interaction of chdir() and
> > async code, but this will at least restore the old behavior in the
> > synchronous case.
> >
> > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
>
> the meta-qcom CI started failing.. and I have the feeling this is
> because of this change. I am now seeing many errors, and this:
> 00:02:31.320 OSError: AF_UNIX path too long
>
> Here is the link to the full Jenkins log:
> https://ci.linaro.org/job/lt-qcom-openembedded-meta-qcom-master-premerge/136/MACHINE=dragonboard-410c,TCLIBC=glibc,label=docker-buster-amd64/console
>
> This is a test build we do for each Github PR. This build is using
> this commit from Poky/master:
> 982688103619 (build-appliance-image: Update to master head revision)
>
> We tested another PR 2 days ago, and it went fine:
> https://ci.linaro.org/job/lt-qcom-openembedded-meta-qcom-master-premerge/134/MACHINE=dragonboard-410c,TCLIBC=glibc,label=docker-buster-amd64/consoleFull
> This build was based on this commit:
> 982688103619 (build-appliance-image: Update to master head revision)
>
> Looking at the error log, and this commit, it looks related to me..
> though I am not sure I understand the details ;)

Argh, this is because I was silly and the fix is outright wrong. Sorry
about that, I'll fix it quick (and maybe add a test this time!)

>
> > ---
> >  bitbake/lib/hashserv/client.py | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/bitbake/lib/hashserv/client.py b/bitbake/lib/hashserv/client.py
> > index ae5875d1b3..7bbf0865d5 100644
> > --- a/bitbake/lib/hashserv/client.py
> > +++ b/bitbake/lib/hashserv/client.py
> > @@ -40,7 +40,7 @@ class AsyncClient(object):
> >
> >          self._connect_sock = connect_sock
> >
> > -    async def _connect(self):
> > +    async def connect(self):
> >          if self.reader is None or self.writer is None:
> >              (self.reader, self.writer) = await self._connect_sock()
> >
> > @@ -62,7 +62,7 @@ class AsyncClient(object):
> >          count = 0
> >          while True:
> >              try:
> > -                await self._connect()
> > +                await self.connect()
> >                  return await proc()
> >              except (
> >                  OSError,
> > @@ -190,7 +190,6 @@ class Client(object):
> >
> >          for call in (
> >              "connect_tcp",
> > -            "connect_unix",
> >              "close",
> >              "get_unihash",
> >              "report_unihash",
> > @@ -209,6 +208,16 @@ class Client(object):
> >
> >          return wrapper
> >
> > +    def connect_unix(self, path):
> > +        # AF_UNIX has path length issues so chdir here to workaround
> > +        cwd = os.getcwd()
> > +        try:
> > +            os.chdir(os.path.dirname(path))
> > +            self.loop.run_until_complete(self.client.connect_unix(path))
> > +            self.loop.run_until_complete(self.client.connect())
> > +        finally:
> > +            os.chdir(cwd)
> > +
> >      @property
> >      def max_chunk(self):
> >          return self.client.max_chunk
> > --
> > 2.29.2
> >
> >
> > 
> >

Re: [bitbake-devel][PATCH] hashserv: client: Fix AF_UNIX path length limits

[Nicolas Dechesne]

On Wed, Dec 9, 2020 at 10:11 PM Joshua Watt <jpewhacker@gmail.com> wrote:
>
> On Wed, Dec 9, 2020 at 3:04 PM Nicolas Dechesne
> <nicolas.dechesne@linaro.org> wrote:
> >
> > hi Joshua,
> >
> >
> > On Wed, Dec 2, 2020 at 8:58 PM Joshua Watt <JPEWhacker@gmail.com> wrote:
> > >
> > > Restores a fix for unix domain socket path length limits when using the
> > > synchronous hash equivalence client that was accidentally removed when
> > > the async client was added.
> > >
> > > Unfortunately, it's much more difficult to fix the same problem when
> > > using the async client directly due to the interaction of chdir() and
> > > async code, but this will at least restore the old behavior in the
> > > synchronous case.
> > >
> > > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
> >
> > the meta-qcom CI started failing.. and I have the feeling this is
> > because of this change. I am now seeing many errors, and this:
> > 00:02:31.320 OSError: AF_UNIX path too long
> >
> > Here is the link to the full Jenkins log:
> > https://ci.linaro.org/job/lt-qcom-openembedded-meta-qcom-master-premerge/136/MACHINE=dragonboard-410c,TCLIBC=glibc,label=docker-buster-amd64/console
> >
> > This is a test build we do for each Github PR. This build is using
> > this commit from Poky/master:
> > 982688103619 (build-appliance-image: Update to master head revision)
> >
> > We tested another PR 2 days ago, and it went fine:
> > https://ci.linaro.org/job/lt-qcom-openembedded-meta-qcom-master-premerge/134/MACHINE=dragonboard-410c,TCLIBC=glibc,label=docker-buster-amd64/consoleFull
> > This build was based on this commit:
> > 982688103619 (build-appliance-image: Update to master head revision)
> >
> > Looking at the error log, and this commit, it looks related to me..
> > though I am not sure I understand the details ;)
>
> Argh, this is because I was silly and the fix is outright wrong. Sorry
> about that, I'll fix it quick (and maybe add a test this time!)

ah, cool! well, the self claim about being silly ;) but it's good we
found it so quickly! I was starting to worry about something wrong in
our infrastructure!
if you need me to run a test, let me know, I can run our CI with your
patch applied..

>
> >
> > > ---
> > >  bitbake/lib/hashserv/client.py | 15 ++++++++++++---
> > >  1 file changed, 12 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/bitbake/lib/hashserv/client.py b/bitbake/lib/hashserv/client.py
> > > index ae5875d1b3..7bbf0865d5 100644
> > > --- a/bitbake/lib/hashserv/client.py
> > > +++ b/bitbake/lib/hashserv/client.py
> > > @@ -40,7 +40,7 @@ class AsyncClient(object):
> > >
> > >          self._connect_sock = connect_sock
> > >
> > > -    async def _connect(self):
> > > +    async def connect(self):
> > >          if self.reader is None or self.writer is None:
> > >              (self.reader, self.writer) = await self._connect_sock()
> > >
> > > @@ -62,7 +62,7 @@ class AsyncClient(object):
> > >          count = 0
> > >          while True:
> > >              try:
> > > -                await self._connect()
> > > +                await self.connect()
> > >                  return await proc()
> > >              except (
> > >                  OSError,
> > > @@ -190,7 +190,6 @@ class Client(object):
> > >
> > >          for call in (
> > >              "connect_tcp",
> > > -            "connect_unix",
> > >              "close",
> > >              "get_unihash",
> > >              "report_unihash",
> > > @@ -209,6 +208,16 @@ class Client(object):
> > >
> > >          return wrapper
> > >
> > > +    def connect_unix(self, path):
> > > +        # AF_UNIX has path length issues so chdir here to workaround
> > > +        cwd = os.getcwd()
> > > +        try:
> > > +            os.chdir(os.path.dirname(path))
> > > +            self.loop.run_until_complete(self.client.connect_unix(path))
> > > +            self.loop.run_until_complete(self.client.connect())
> > > +        finally:
> > > +            os.chdir(cwd)
> > > +
> > >      @property
> > >      def max_chunk(self):
> > >          return self.client.max_chunk
> > > --
> > > 2.29.2
> > >
> > >
> > > 
> > >