Repository: zeppelin Updated Branches: refs/heads/master 6caf587e1 -> 5d09a7f83
ZEPPELIN-3112: Markdown interpreter fails with NPE ### What is this PR for? Since pegdown-parser is not thread-safe while trying to run multiple MarkDown paragraphs at once, sometimes it fails to render HTML. Ref: https://github.com/sirthias/pegdown/blob/master/src/main/java/org/pegdown/PegDownProcessor.java#L32 ### What type of PR is it? [Improvement] ### What is the Jira issue? * [ZEPPELIN-3112](https://issues.apache.org/jira/browse/ZEPPELIN-3112) ### How should this be tested? * This happens rarely, when you try to run all paragraph from UI which has more the 5-6 `%md` paragraph. This is hard to reproduce in 0.8.0, but can easily be done via 0.7.3. Also, have added a sample [notebook](https://issues.apache.org/jira/secure/attachment/12903037/Test%20MD%20fail.json) in the parent JIRA * Have added test case to verify. ### Questions: * Does the licenses files need update? N/A * Is there breaking changes for older versions? N/A * Does this needs documentation? N/A Author: Prabhjyot Singh <prabhjyotsi...@gmail.com> Closes #2711 from prabhjyotsingh/ZEPPELIN-3112 and squashes the following commits: e796e52cd [Prabhjyot Singh] ZEPPELIN-3112: call markdownToHtml in synchronized block Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/5d09a7f8 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/5d09a7f8 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/5d09a7f8 Branch: refs/heads/master Commit: 5d09a7f836ebaddd40e644ef82d80320570364aa Parents: 6caf587 Author: Prabhjyot Singh <prabhjyotsi...@gmail.com> Authored: Wed Dec 20 18:09:04 2017 +0530 Committer: Prabhjyot Singh <prabhjyotsi...@gmail.com> Committed: Fri Dec 22 12:40:53 2017 +0530 ---------------------------------------------------------------------- .../apache/zeppelin/markdown/PegdownParser.java | 6 ++- .../zeppelin/markdown/PegdownParserTest.java | 39 ++++++++++++++++++-- 2 files changed, 39 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5d09a7f8/markdown/src/main/java/org/apache/zeppelin/markdown/PegdownParser.java ---------------------------------------------------------------------- diff --git a/markdown/src/main/java/org/apache/zeppelin/markdown/PegdownParser.java b/markdown/src/main/java/org/apache/zeppelin/markdown/PegdownParser.java index baf18f0..fb99f05 100644 --- a/markdown/src/main/java/org/apache/zeppelin/markdown/PegdownParser.java +++ b/markdown/src/main/java/org/apache/zeppelin/markdown/PegdownParser.java @@ -41,8 +41,10 @@ public class PegdownParser implements MarkdownParser { @Override public String render(String markdownText) { String html = ""; - String parsed = processor.markdownToHtml(markdownText); - + String parsed; + synchronized (processor) { + parsed = processor.markdownToHtml(markdownText); + } if (null == parsed) { throw new RuntimeException("Cannot parse markdown text to HTML using pegdown"); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5d09a7f8/markdown/src/test/java/org/apache/zeppelin/markdown/PegdownParserTest.java ---------------------------------------------------------------------- diff --git a/markdown/src/test/java/org/apache/zeppelin/markdown/PegdownParserTest.java b/markdown/src/test/java/org/apache/zeppelin/markdown/PegdownParserTest.java index 0c545dc..2e1d857 100644 --- a/markdown/src/test/java/org/apache/zeppelin/markdown/PegdownParserTest.java +++ b/markdown/src/test/java/org/apache/zeppelin/markdown/PegdownParserTest.java @@ -19,6 +19,7 @@ package org.apache.zeppelin.markdown; import static org.junit.Assert.assertEquals; +import java.util.ArrayList; import java.util.Properties; import org.apache.zeppelin.interpreter.InterpreterResult; @@ -26,10 +27,8 @@ import static org.apache.zeppelin.markdown.PegdownParser.wrapWithMarkdownClassDi import static org.junit.Assert.assertThat; import org.hamcrest.CoreMatchers; -import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; +import org.junit.*; +import org.junit.rules.ErrorCollector; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,6 +36,9 @@ public class PegdownParserTest { Logger logger = LoggerFactory.getLogger(PegdownParserTest.class); Markdown md; + @Rule + public ErrorCollector collector = new ErrorCollector(); + @Before public void setUp() throws Exception { Properties props = new Properties(); @@ -51,6 +53,35 @@ public class PegdownParserTest { } @Test + public void testMultipleThread() { + ArrayList<Thread> arrThreads = new ArrayList<Thread>(); + for (int i = 0; i < 10; i++) { + Thread t = new Thread() { + public void run() { + String r1 = null; + try { + r1 = md.interpret("# H1", null).code().name(); + } catch (Exception e) { + logger.error("testTestMultipleThread failed to interpret", e); + } + collector.checkThat("SUCCESS", + CoreMatchers.containsString(r1)); + } + }; + t.start(); + arrThreads.add(t); + } + + for (int i = 0; i < 10; i++) { + try { + arrThreads.get(i).join(); + } catch (InterruptedException e) { + logger.error("testTestMultipleThread failed to join threads", e); + } + } + } + + @Test public void testHeader() { InterpreterResult r1 = md.interpret("# H1", null); assertEquals(wrapWithMarkdownClassDiv("<h1>H1</h1>"), r1.message().get(0).getData());
