diff --git a/core/src/main/java/com/taobao/arthas/core/shell/system/impl/ProcessImpl.java b/core/src/main/java/com/taobao/arthas/core/shell/system/impl/ProcessImpl.java index 9126eb030c7..22c0729bb84 100644 --- a/core/src/main/java/com/taobao/arthas/core/shell/system/impl/ProcessImpl.java +++ b/core/src/main/java/com/taobao/arthas/core/shell/system/impl/ProcessImpl.java @@ -21,6 +21,7 @@ import com.taobao.arthas.core.shell.system.Process; import com.taobao.arthas.core.shell.system.ProcessAware; import com.taobao.arthas.core.shell.term.Tty; +import com.taobao.middleware.cli.CLI; import com.taobao.middleware.cli.CLIException; import com.taobao.middleware.cli.CommandLine; import io.termd.core.function.Function; @@ -346,15 +347,24 @@ public synchronized void run(boolean fg) { } CommandLine cl = null; + ResultModel helpResult = null; try { - if (commandContext.cli() != null) { - if (commandContext.cli().parse(args2, false).isAskingForHelp()) { - appendResult(new HelpCommand().createHelpDetailModel(commandContext)); + CLI cli = commandContext.cli(); + if (cli != null) { + synchronized (cli) { + if (cli.parse(args2, false).isAskingForHelp()) { + helpResult = new HelpCommand().createHelpDetailModel(commandContext); + } else { + cl = cli.parse(args2); + } + } + + if (helpResult != null) { + appendResult(helpResult); terminate(); return; } - cl = commandContext.cli().parse(args2); process.setArgs2(args2); process.setCommandLine(cl); } diff --git a/core/src/test/java/com/taobao/arthas/core/shell/system/impl/ProcessImplConcurrencyTest.java b/core/src/test/java/com/taobao/arthas/core/shell/system/impl/ProcessImplConcurrencyTest.java index 985eacc3099..932fa7a8fb1 100644 --- a/core/src/test/java/com/taobao/arthas/core/shell/system/impl/ProcessImplConcurrencyTest.java +++ b/core/src/test/java/com/taobao/arthas/core/shell/system/impl/ProcessImplConcurrencyTest.java @@ -1,12 +1,17 @@ package com.taobao.arthas.core.shell.system.impl; +import com.taobao.arthas.core.command.monitor200.MonitorCommand; import com.taobao.arthas.core.command.model.MessageModel; import com.taobao.arthas.core.command.model.ResultModel; import com.taobao.arthas.core.command.view.ResultView; import com.taobao.arthas.core.command.view.ResultViewResolver; +import com.taobao.arthas.core.distribution.ResultDistributor; import com.taobao.arthas.core.distribution.impl.TermResultDistributorImpl; +import com.taobao.arthas.core.shell.cli.CliTokens; +import com.taobao.arthas.core.shell.command.Command; import com.taobao.arthas.core.shell.command.CommandProcess; import com.taobao.arthas.core.shell.handlers.Handler; +import com.taobao.arthas.core.shell.handlers.NoOpHandler; import com.taobao.arthas.core.shell.term.Tty; import com.taobao.arthas.core.shell.system.ExecStatus; import com.taobao.arthas.core.shell.system.Process; @@ -16,13 +21,73 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Field; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; public class ProcessImplConcurrencyTest { + private static final ResultDistributor DISCARD_RESULT_DISTRIBUTOR = new ResultDistributor() { + @Override + public void appendResult(ResultModel result) { + } + + @Override + public void close() { + } + }; + + @Test + public void testRunShouldSerializeSharedCliParsing() throws Exception { + final Command sharedCommand = Command.create(MonitorCommand.class); + final int workerCount = 8; + final int iterations = 1000; + final CountDownLatch start = new CountDownLatch(1); + ExecutorService executor = Executors.newFixedThreadPool(workerCount); + List> workers = new ArrayList>(workerCount); + + try { + for (int worker = 0; worker < workerCount; worker++) { + workers.add(executor.submit(new Runnable() { + @Override + public void run() { + try { + start.await(); + for (int iteration = 0; iteration < iterations; iteration++) { + Tty tty = new MockTty(); + ProcessImpl process = new ProcessImpl(sharedCommand, + Collections.singletonList(CliTokens.createText("-h")), + new NoOpHandler(), + new ProcessImpl.ProcessOutput(Collections.>emptyList(), null, tty), + DISCARD_RESULT_DISTRIBUTOR); + process.setTty(tty); + process.run(false); + Assert.assertEquals(ExecStatus.TERMINATED, process.status()); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError(e); + } + } + })); + } + + start.countDown(); + for (Future worker : workers) { + worker.get(10, TimeUnit.SECONDS); + } + } finally { + executor.shutdownNow(); + Assert.assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS)); + } + } + @Test public void testAppendResultShouldNotDeadlockWithProcessMonitor() throws Exception { ProcessImpl processImpl = new ProcessImpl(null, Collections.emptyList(), null,