-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18773/#review36234
-----------------------------------------------------------



core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java
<https://reviews.apache.org/r/18773/#comment67175>

    Should move the assignment inside of the try-block.



core/src/test/java/org/apache/accumulo/core/util/shell/ShellTest.java
<https://reviews.apache.org/r/18773/#comment67176>

    Do we need a null check?



test/src/test/java/org/apache/accumulo/test/ShellServerIT.java
<https://reviews.apache.org/r/18773/#comment67174>

    Why did the tracer get removed from here?



test/src/test/java/org/apache/accumulo/test/ShellServerIT.java
<https://reviews.apache.org/r/18773/#comment67177>

    Do we need a null check?


- Mike Drob


On March 5, 2014, 4:29 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18773/
> -----------------------------------------------------------
> 
> (Updated March 5, 2014, 4:29 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2429
>     https://issues.apache.org/jira/browse/ACCUMULO-2429
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> The JLine 2 ConsoleReader used by Shell spawns a thread which should be 
> cleaned up when done with the Shell. Otherwise, the thread leaks, taking up 
> resources when the shell is used programmatically. This commit adds a 
> shutdown() method to Shell for cleaning up the thread. This enables 
> ShellServerIT to pass reliably and not flood the OS with leaked threads.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java 
> 850816c7e1673d89ae8fd29818cf3322c3cd8b3b 
>   core/src/test/java/org/apache/accumulo/core/util/shell/ShellConfigTest.java 
> df4d817aad50e3c34e6de02f68b4345af00bfac6 
>   
> core/src/test/java/org/apache/accumulo/core/util/shell/ShellSetInstanceTest.java
>  5a6cc8a0052b03cfa510b80b072408139428ccff 
>   core/src/test/java/org/apache/accumulo/core/util/shell/ShellTest.java 
> bf203f77d4e6809bacdb82217e3c8d8aaeecbe48 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 
> da094e85ed3ebaeef2ac2cac5beb7b52299a49dc 
> 
> Diff: https://reviews.apache.org/r/18773/diff/
> 
> 
> Testing
> -------
> 
> Ran shell-related unit tests successfully. Ran ShellServerIT successfully 
> under both Mac OS X and CentOS 6. Ran shell against live instance to ensure 
> it starts and stops normally.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>

Reply via email to