Re: [PATCH] [edk2/BaseTools] edksetup.bat stuck on unicode locale Windows

["Park, Aiden" <aiden.park@intel.com>]

Hi Bob,

> -----Original Message-----
> From: Feng, Bob C <bob.c.feng@intel.com>
> Sent: Thursday, December 19, 2019 10:12 PM
> To: Park, Aiden <aiden.park@intel.com>; devel@edk2.groups.io
> Subject: RE: [PATCH] [edk2/BaseTools] edksetup.bat stuck on unicode locale
> Windows
> 
> Hi Aiden,
> 
> I'd like to know why need to call 'edksetup.bat forcerebuild' with python
> subprocess.call().
> Is there other way to execute edksetup.bat?
> 
I forgot to mention one more thing in reproduce steps. This is also reproducible by just calling 'edksetup.bat forcerebuild' in Windows command prompt w/o python subprocess.call().
I have tested on 'Chinese - Traditional, Taiwan' and 'Korean' locale Windows and it's reproducible on both Unicode locales. And below change works around the issue on both locales.

Some experiment. Not sure if this is python2 bug.
message = "" <= type 'str'
temp = stdout.decode(encoding='utf-8', errors='ignore') <= type 'unicode' which has unicode string
message = temp.encode('utf-8') <= pass
message = temp <= deadlock

> Thanks,
> Bob
> -----Original Message-----
> From: Park, Aiden
> Sent: Friday, December 20, 2019 8:23 AM
> To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io
> Subject: RE: [PATCH] [edk2/BaseTools] edksetup.bat stuck on unicode locale
> Windows
> 
> Hi Bob,
> 
> > -----Original Message-----
> > From: Feng, Bob C <bob.c.feng@intel.com>
> > Sent: Tuesday, December 17, 2019 9:23 PM
> > To: devel@edk2.groups.io; Park, Aiden <aiden.park@intel.com>
> > Cc: Feng, Bob C <bob.c.feng@intel.com>
> > Subject: RE: [PATCH] [edk2/BaseTools] edksetup.bat stuck on unicode
> > locale Windows
> >
> > Hi Aiden,
> >
> > If set kwargs["stdout"] = sys.stdout, then stdout and stderr messages
> > from sub process will directly write to sys.stdout.
> > I think this patch works because sys.stdout.encoding is the correct
> encoding.
> > So what do you think if we fix this Non-Ascii issue by Changing the
> > line of message = stdout.decode(encoding='utf-8', errors='ignore')
> > #for compatibility in python 2 and 3 to message =
> > stdout.decode(encoding=sys.stdout.encoding , errors='ignore') #for
> > compatibility in python 2 and 3
> >
> > Otherwise, I think this patch may need to take care of the code after
> > this line kwargs["stdout"] = sys.stdout, I mean
> > p.communicate() will return empty string after your change, and the
> > following code would be useless.
> >
> Thanks for your comment. You are right. This change makes the following
> code useless. It should use PIPE.
> 
> I did double-check the environment to make sure reproduce steps, and it
> turns out that it's reproducible on Unicode locale Windows + Python 2.
> Python 3 does not have the issue.
> 
> There seems to be 2 locations which make deadlock.
> 1. subprocess.Popen()
> It looks there is a race condition when creating a child process with PIPE.
> Therefore, a lock is put for Popen().
> +    popen_lock.acquire(True)
>      p = subprocess.Popen(Args, cwd=WorkDir, stderr=kwargs["stderr"],
> stdout=kwargs["stdout"])
> +    popen_lock.release()
> 
> 2. stdout.decode()
> stdout variable is decoded to 'unicode' type message variable. But, it's stuck
> if stdout has Unicode string even if errors='ignore'.
> Converting The 'unicode' type message to 'str' type message resolves the
> issue.
> -        message = stdout.decode(encoding='utf-8', errors='ignore') #for
> compatibility in python 2 and 3
> +        message = stdout.decode(encoding='utf-8',
> + errors='ignore').encode('utf-8') #for compatibility in python 2 and 3
> 
> FYI, your recommendation encoding=sys.stdout.encoding does not help to
> resolve this issue.
> 
> I look forward to your feedback. Thanks.
> 
> > Thanks,
> > Bob
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Park, Aiden
> > Sent: Thursday, December 12, 2019 6:43 AM
> > To: devel@edk2.groups.io
> > Subject: [edk2-devel] [PATCH] [edk2/BaseTools] edksetup.bat stuck on
> > unicode locale Windows
> >
> > This issue happens under two conditions.
> >   1. Unicode language environment in Windows
> >   2. Call 'edksetup.bat forcerebuild' with python subprocess.call()
> >
> > Steps to reproduce
> >   C:\edk2>python
> >   Python 2.7.12 (v2.7.12:d33e0cf91556, Jun 27 2016, 15:24:40)
> >   Type help, copyright, credits or license for more information.
> >   >>> import subprocess
> >   >>> subprocess.call(['edksetup.bat', 'forcerebuild'])
> >
> >   The edksetup.bat stuck at 'nmake cleanall'.
> >
> > One of multi-threads is on deadlock when python handles stdout and
> > stderr in a subprocess pipe only if the outputs include unicode chars.
> > Only stderr will be handled in the pipe same as a single thread call.
> >
> > Reported in Slim Bootloader.
> >   https://github.com/slimbootloader/slimbootloader/issues/478
> > Local fix has been made in Slim Bootloader.
> >   https://github.com/slimbootloader/slimbootloader/pull/490
> >
> > Signed-off-by: Aiden Park <aiden.park@intel.com>
> > ---
> >  BaseTools/Source/C/Makefiles/NmakeSubdirs.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/BaseTools/Source/C/Makefiles/NmakeSubdirs.py
> > b/BaseTools/Source/C/Makefiles/NmakeSubdirs.py
> > index 356f5ac..c77bfb0 100644
> > --- a/BaseTools/Source/C/Makefiles/NmakeSubdirs.py
> > +++ b/BaseTools/Source/C/Makefiles/NmakeSubdirs.py
> > @@ -33,7 +33,7 @@ def RunCommand(WorkDir=None, *Args, **kwargs):
> >      if "stderr" not in kwargs:
> >          kwargs["stderr"] = subprocess.STDOUT
> >      if "stdout" not in kwargs:
> > -        kwargs["stdout"] = subprocess.PIPE
> > +        kwargs["stdout"] = sys.stdout
> >      p = subprocess.Popen(Args, cwd=WorkDir, stderr=kwargs["stderr"],
> > stdout=kwargs["stdout"])
> >      stdout, stderr = p.communicate()
> >      message = ""
> > --
> > 2.10.2.windows.1
> >
> > 
> >
> 
> Best Regards,
> Aiden
> 

Best Regards,
Aiden