You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by pr...@apache.org on 2017/12/22 07:10:58 UTC
zeppelin git commit: ZEPPELIN-3112: Markdown interpreter fails with
NPE
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 <pr...@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 <pr...@gmail.com>
Authored: Wed Dec 20 18:09:04 2017 +0530
Committer: Prabhjyot Singh <pr...@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());