[GitHub] [beam] scwhittle commented on pull request #12825: [BEAM-9399] Change DataflowWorkerLoggingHandler to report errors to t…

[GitBox <gi...@apache.org>]

scwhittle commented on pull request #12825:
URL: https://github.com/apache/beam/pull/12825#issuecomment-691048364


   @lukecwik Luke can you review as you have context from last time?
   
   Changing to synchronizing on buffer instead of the PrintStream changes the precondition to just enforce the invariant on when publish is called within our custom PrintStream. That means that the original deadlock can still occur if this happens:
   
   T1: synchronizes on System.err (Throwable.printStackTrace for example), publishes to handler
   T2: synchronized within handler, tries to report error to System.err using Throwable.printStackTrace which synchronizes on the PrintStream
   
   I removed that case by using the custom ErrorManager to print to the original stderr stream, an alternative would be to change the ErrorManager still use our custom PrintStream but remove use of Throwable.printStackTrace so as not to sychronize on the PrintStream.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org