[ https://issues.apache.org/jira/browse/STORM-442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14196038#comment-14196038 ]
ASF GitHub Bot commented on STORM-442: -------------------------------------- Github user clockfly commented on a diff in the pull request: https://github.com/apache/storm/pull/305#discussion_r19798787 --- Diff: storm-core/src/jvm/backtype/storm/utils/ShellProcess.java --- @@ -135,7 +135,14 @@ public void logErrorStream() { public String getErrorsString() { if (processErrorStream != null) { try { - return IOUtils.toString(processErrorStream); + StringBuilder sb = new StringBuilder(); + while (processErrorStream.available() > 0) { + int bufferSize = processErrorStream.available(); --- End diff -- It is not safe to use available(). Check http://stackoverflow.com/questions/804951/is-it-possible-to-read-from-a-inputstream-with-a-timeout > multilang ShellBolt/ShellSpout die() can be hang when Exception happened > ------------------------------------------------------------------------ > > Key: STORM-442 > URL: https://issues.apache.org/jira/browse/STORM-442 > Project: Apache Storm > Issue Type: Bug > Affects Versions: 0.9.3 > Reporter: DashengJu > > In ShellBolt, the _readerThread read command from python/shell process, and > handle like this: > try { > ShellMsg shellMsg = _process.readShellMsg(); > ... > } catch (InterruptedException e) { > } catch (Throwable t) { > die(t); > } > And in the die function, getProcessTerminationInfoString will read > getErrorsString() from processErrorStream. > private void die(Throwable exception) { > > String processInfo = _process.getProcessInfoString() + > _process.getProcessTerminationInfoString(); > > _exception = new RuntimeException(processInfo, exception); > > } > so when ShellBolt got exception(for example, readShellMsg() throw NPE ) , > but it is not an error from sub process, then > getProcessTerminationInfoString will be hang because processErrorStream have > no data to read. > On the other hand, as [~xiaokang] says ShellBolt should fail fast on > exception ( https://github.com/apache/incubator-storm/pull/46 ) , I think it > is not a good idea to read error info from stream. > Because [~xiaokang] 's PR is based old version, so I will move his code to > this PR, and modify some other place in ShellSpout. -- This message was sent by Atlassian JIRA (v6.3.4#6332)