Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Redcarpet::Markdown#render thread safe #672

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 30 additions & 2 deletions ext/redcarpet/rc_markdown.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,17 +125,22 @@ static VALUE rb_redcarpet_md__new(int argc, VALUE *argv, VALUE klass)

rb_markdown = Data_Wrap_Struct(klass, NULL, rb_redcarpet_md__free, markdown);
rb_iv_set(rb_markdown, "@renderer", rb_rndr);
rb_iv_set(rb_markdown, "@render_lock", rb_mutex_new());

return rb_markdown;
}

static VALUE rb_redcarpet_md_render(VALUE self, VALUE text)
static VALUE rb_redcarpet_md_render_begin(VALUE rb_args)
{
VALUE self;
VALUE text;
VALUE rb_rndr;
VALUE rb_render_lock;
struct buf *output_buf;
struct sd_markdown *markdown;

Check_Type(text, T_STRING);
self = rb_ary_entry(rb_args, 0);
text = rb_ary_entry(rb_args, 1);

rb_rndr = rb_iv_get(self, "@renderer");
Data_Get_Struct(self, struct sd_markdown, markdown);
Expand Down Expand Up @@ -170,6 +175,29 @@ static VALUE rb_redcarpet_md_render(VALUE self, VALUE text)
return text;
}

static VALUE rb_redcarpet_md_ensure(VALUE rb_args)
{
VALUE self;

self = rb_ary_entry(rb_args, 0);
rb_mutex_unlock(rb_iv_get(self, "@render_lock"));
rb_ary_free(rb_args);
return Qnil;
}

static VALUE rb_redcarpet_md_render(VALUE self, VALUE text)
{
VALUE rb_args;

Check_Type(text, T_STRING);
// rb_ensure requires a single VALUE to the callbacks so we turn our two arguments into an array
rb_args = rb_ary_new_from_args(2, self, text);

// sd_markdown and the rendering methods are not thread safe so wrap all rendering in a lock
rb_mutex_lock(rb_iv_get(self, "@render_lock"));
return rb_ensure(&rb_redcarpet_md_render_begin, rb_args, &rb_redcarpet_md_ensure, rb_args);
}

__attribute__((visibility("default")))
void Init_redcarpet()
{
Expand Down
41 changes: 41 additions & 0 deletions test/concurrent_render_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# coding: UTF-8
require 'test_helper'

class ConcurrentRenderTest < Redcarpet::TestCase
class SlowRender < Redcarpet::Render::Base
def paragraph(text)
sleep 0.01
text
end
end

def test_concurrent_render
markdown = Redcarpet::Markdown.new(SlowRender)
threads = (1..5).map do
Thread.new do
5.times do
assert_equal 'hi', markdown.render('hi')
end
end
end
threads.each(&:join)
end

class BadRenderException < StandardError; end

class ExceptionRender < Redcarpet::Render::Base
def paragraph(text)
raise BadRenderException
end
end

def test_lock_released_on_exception
markdown = Redcarpet::Markdown.new(ExceptionRender)
assert_raises BadRenderException do
markdown.render('hi')
end
assert_raises BadRenderException do
markdown.render('hi')
end
end
end