* [PATCH] BaseTools/Python: Add missing FatalError handling @ 2020-05-27 19:41 Name 2020-05-28 19:23 ` Irene Park 0 siblings, 1 reply; 4+ messages in thread From: Name @ 2020-05-27 19:41 UTC (permalink / raw) To: devel; +Cc: Irene Park From: Irene Park <ipark@nvidia.com> AutoGenWorker doesn't handle the exception from FatalError therefore the build fails to return the proper error code at the exit. Signed-off-by: Irene Park <ipark@nvidia.com> --- BaseTools/Source/Python/AutoGen/AutoGenWorker.py | 6 ++++++ BaseTools/Source/Python/build/build.py | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py index 563d91b..2395964 100755 --- a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py +++ b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py @@ -24,6 +24,7 @@ import traceback import sys from AutoGen.DataPipe import MemoryDataPipe import logging +from Common.BuildToolError import FatalError def clearQ(q): try: @@ -101,6 +102,7 @@ class AutoGenManager(threading.Thread): self.autogen_workers = autogen_workers self.feedback_q = feedback_q self.Status = True + self.Error = 0 self.error_event = error_event def run(self): try: @@ -113,6 +115,7 @@ class AutoGenManager(threading.Thread): fin_num += 1 else: self.Status = False + self.Error = self.feedback_q.get() self.TerminateWorkers() if fin_num == len(self.autogen_workers): self.clearQueue() @@ -282,6 +285,9 @@ class AutoGenWorkerInProcess(mp.Process): except Empty: pass + except FatalError as e: + self.feedback_q.put(taskname) + self.feedback_q.put(e.args[0]) except: self.feedback_q.put(taskname) finally: diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py index ed3a3b9..d6e3d84 100755 --- a/BaseTools/Source/Python/build/build.py +++ b/BaseTools/Source/Python/build/build.py @@ -880,7 +880,10 @@ class Build(): self.AutoGenMgr.join() rt = self.AutoGenMgr.Status - return rt, 0 + err = 0 + if not rt: + err = self.AutoGenMgr.Error + return rt, err except FatalError as e: return False, e.args[0] except: -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] BaseTools/Python: Add missing FatalError handling 2020-05-27 19:41 [PATCH] BaseTools/Python: Add missing FatalError handling Name @ 2020-05-28 19:23 ` Irene Park 2020-05-29 1:08 ` Bob Feng 0 siblings, 1 reply; 4+ messages in thread From: Irene Park @ 2020-05-28 19:23 UTC (permalink / raw) To: devel@edk2.groups.io; +Cc: bob.c.feng@intel.com, liming.gao@intel.com A gentle reminder and adding Bob Feng and Liming Gao to CC. Thank you, Irene -----Original Message----- From: Irene Park <ipark@nvidia.com> Sent: Wednesday, May 27, 2020 3:42 PM To: devel@edk2.groups.io Cc: Irene Park <ipark@nvidia.com> Subject: [PATCH] BaseTools/Python: Add missing FatalError handling From: Irene Park <ipark@nvidia.com> AutoGenWorker doesn't handle the exception from FatalError therefore the build fails to return the proper error code at the exit. Signed-off-by: Irene Park <ipark@nvidia.com> --- BaseTools/Source/Python/AutoGen/AutoGenWorker.py | 6 ++++++ BaseTools/Source/Python/build/build.py | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py index 563d91b..2395964 100755 --- a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py +++ b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py @@ -24,6 +24,7 @@ import traceback import sys from AutoGen.DataPipe import MemoryDataPipe import logging +from Common.BuildToolError import FatalError def clearQ(q): try: @@ -101,6 +102,7 @@ class AutoGenManager(threading.Thread): self.autogen_workers = autogen_workers self.feedback_q = feedback_q self.Status = True + self.Error = 0 self.error_event = error_event def run(self): try: @@ -113,6 +115,7 @@ class AutoGenManager(threading.Thread): fin_num += 1 else: self.Status = False + self.Error = self.feedback_q.get() self.TerminateWorkers() if fin_num == len(self.autogen_workers): self.clearQueue() @@ -282,6 +285,9 @@ class AutoGenWorkerInProcess(mp.Process): except Empty: pass + except FatalError as e: + self.feedback_q.put(taskname) + self.feedback_q.put(e.args[0]) except: self.feedback_q.put(taskname) finally: diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py index ed3a3b9..d6e3d84 100755 --- a/BaseTools/Source/Python/build/build.py +++ b/BaseTools/Source/Python/build/build.py @@ -880,7 +880,10 @@ class Build(): self.AutoGenMgr.join() rt = self.AutoGenMgr.Status - return rt, 0 + err = 0 + if not rt: + err = self.AutoGenMgr.Error + return rt, err except FatalError as e: return False, e.args[0] except: -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] BaseTools/Python: Add missing FatalError handling 2020-05-28 19:23 ` Irene Park @ 2020-05-29 1:08 ` Bob Feng 2020-05-29 7:27 ` [edk2-devel] " Irene Park 0 siblings, 1 reply; 4+ messages in thread From: Bob Feng @ 2020-05-29 1:08 UTC (permalink / raw) To: Irene Park, devel@edk2.groups.io; +Cc: Gao, Liming Hi Irene, For this piece of code + self.feedback_q.put(taskname) + self.feedback_q.put(e.args[0]) I think there will be the case that another autogenworker process insert its taskname before the current autogenworker process do self.feedback_q.put(e.args[0]). If that case happens, the build tool will hang. Thanks, Bob -----Original Message----- From: Irene Park <ipark@nvidia.com> Sent: Friday, May 29, 2020 3:24 AM To: devel@edk2.groups.io Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com> Subject: RE: [PATCH] BaseTools/Python: Add missing FatalError handling A gentle reminder and adding Bob Feng and Liming Gao to CC. Thank you, Irene -----Original Message----- From: Irene Park <ipark@nvidia.com> Sent: Wednesday, May 27, 2020 3:42 PM To: devel@edk2.groups.io Cc: Irene Park <ipark@nvidia.com> Subject: [PATCH] BaseTools/Python: Add missing FatalError handling From: Irene Park <ipark@nvidia.com> AutoGenWorker doesn't handle the exception from FatalError therefore the build fails to return the proper error code at the exit. Signed-off-by: Irene Park <ipark@nvidia.com> --- BaseTools/Source/Python/AutoGen/AutoGenWorker.py | 6 ++++++ BaseTools/Source/Python/build/build.py | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py index 563d91b..2395964 100755 --- a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py +++ b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py @@ -24,6 +24,7 @@ import traceback import sys from AutoGen.DataPipe import MemoryDataPipe import logging +from Common.BuildToolError import FatalError def clearQ(q): try: @@ -101,6 +102,7 @@ class AutoGenManager(threading.Thread): self.autogen_workers = autogen_workers self.feedback_q = feedback_q self.Status = True + self.Error = 0 self.error_event = error_event def run(self): try: @@ -113,6 +115,7 @@ class AutoGenManager(threading.Thread): fin_num += 1 else: self.Status = False + self.Error = self.feedback_q.get() self.TerminateWorkers() if fin_num == len(self.autogen_workers): self.clearQueue() @@ -282,6 +285,9 @@ class AutoGenWorkerInProcess(mp.Process): except Empty: pass + except FatalError as e: + self.feedback_q.put(taskname) + self.feedback_q.put(e.args[0]) except: self.feedback_q.put(taskname) finally: diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py index ed3a3b9..d6e3d84 100755 --- a/BaseTools/Source/Python/build/build.py +++ b/BaseTools/Source/Python/build/build.py @@ -880,7 +880,10 @@ class Build(): self.AutoGenMgr.join() rt = self.AutoGenMgr.Status - return rt, 0 + err = 0 + if not rt: + err = self.AutoGenMgr.Error + return rt, err except FatalError as e: return False, e.args[0] except: -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] BaseTools/Python: Add missing FatalError handling 2020-05-29 1:08 ` Bob Feng @ 2020-05-29 7:27 ` Irene Park 0 siblings, 0 replies; 4+ messages in thread From: Irene Park @ 2020-05-29 7:27 UTC (permalink / raw) To: Bob Feng, devel [-- Attachment #1: Type: text/plain, Size: 1517 bytes --] Hi Bob, Thanks for the review. Here is the original description of the issue. There has been a build failure from SCT, related to https://edk2.groups.io/g/devel/message/60407?p=,,,20,0,0,0::relevance,,gEfiFormBrowserExProtocolGuid,20,2,0,74529350. ( https://edk2.groups.io/g/devel/message/60407?p=,,,20,0,0,0::relevance,,gEfiFormBrowserExProtocolGuid,20,2,0,74529350 ) The build log printed the error code obviously but returned with sys.exit(0). Unfortunately this behavior confused the build system and made me notice the failure so late. If there is a concern of the conflict among multiple autogenworkers, I wonder how you think this way. 1. No change in BaseTools/Source/Python/AutoGen/AutoGenWorker.py 2. BaseTools/Source/Python/build/build.py --- a/BaseTools/Source/Python/build/build.py +++ b/BaseTools/Source/Python/build/build.py @@ -880,7 +880,10 @@ class Build(): self.AutoGenMgr.join() rt = self.AutoGenMgr.Status - return rt, 0 + err = 0 + if not rt: + err = UNKNOWN_ERROR << Use a fixed error type of FatalError instead of getting the exact error type from the autogenworkers. + return rt, err except FatalError as e: return False, e.args[0] except: In this way, no change in the multi-thread operation and build.py is able to exit with a standard default error. It's not needed to catch the exact error type. Please share your opinion. Thanks, Irene [-- Attachment #2: Type: text/html, Size: 2083 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-29 7:27 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-05-27 19:41 [PATCH] BaseTools/Python: Add missing FatalError handling Name 2020-05-28 19:23 ` Irene Park 2020-05-29 1:08 ` Bob Feng 2020-05-29 7:27 ` [edk2-devel] " Irene Park
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox