[jira] [Commented] (DAFFODIL-2778) Make the CLI thread-safe

["Steve Lawrence (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/DAFFODIL-2778?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17680731#comment-17680731 ] 

Steve Lawrence commented on DAFFODIL-2778:
------------------------------------------

There are three main issues I've found when trying to make the CLI thread safe:

h2. 1. stdin, stdout, stderr

When the CLI runs, it mutates STDIN, STDOUT, and STDERR variables to control where it writes. This is needed to ensure that tests use special buffers instead of the normal System.in/out/err and so the the CLI input/output can be validated. These variables are mutated in the Main object, and so are not thread safe.

One solution is to allow the creation of new Main instances that accept the input/outputs, like this:

{code:scala}
object Main {
  def main(arguments: Array[String]): Unit = {
    val main = new Main()
    val exitCode = main.run(arguments)
    System.exit(exitCode.id)
}

class Main(
  stdin: InputStream = System.in,
  stdout: PrintStream = System.out,
  stderr: PrintStream = System.err,
) {
  // existing main logic here
}
{code}

This way things that need to be thread safe or use different input/output can create a new Main instance passing in custom stdin/out/err and it's all thread safe.

h2. 2. Log4j capture

The CLI tests need the ability to capture and validation log4j output. But Log4j in tests is configured to go to a single OutputStreamAppender, which is changed for each test to output to a buffer that the CLI tests can read. This is not thread safe.

One solution is to create a custom Log4j appender. This appender could have a ThreadLocal[PrintStream]. Each time we start a test, we set the thread local to the buffer for that test, and each time log4j logs something, we get the ThreadLocal value and write to that stream. This way each thread writes to a thread specific buffer PrintStream.

h2. 3. Log4j Log Level

The CLI currently changes the log level via the global Configurator.setLevel. This means that if CLI tests were run in parallel, the log level could randomly change if another CLI tests starts and changes the level, which can cause failures in tests that expect certain logging information.

Similar to 2, we need some sort of thread specific log level. I don't think Log4j supports this--it is unclear if this is possible, and and I think is the biggest issue blocking this. Maybe a different logging infrastructure is needed...



> Make the CLI thread-safe
> ------------------------
>
>                 Key: DAFFODIL-2778
>                 URL: https://issues.apache.org/jira/browse/DAFFODIL-2778
>             Project: Daffodil
>          Issue Type: Bug
>          Components: CLI
>    Affects Versions: 3.5.0
>            Reporter: Steve Lawrence
>            Priority: Minor
>
> The CLI is not currently thread safe. In most cases this isn't a big deal because the CLI is wrong from a command line and so each instance of daffodil runs in its own process space and we don't have to worry about thread safety.
> However, when testing the CLI, we run it in separate threads instead of forking, which substantially decreases the time needed to run tests. But because the CLI isn't thread safe, we must disable parallel execution of tests in daffodil-cli/build.sbt.
> Ideally we could modify the CLI so it was thread safe, which would allow parallel test execution. It would also also libraries to run the CLI without forking if it was something they needed.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)