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

Assertion failed: `(md->work_bufs[BUFFER_SPAN].size == 0)` - Thread safety related? #570

Open
boazsegev opened this Issue Aug 7, 2016 · 0 comments

Comments

Projects
None yet
1 participant
@boazsegev

boazsegev commented Aug 7, 2016

First off, thanks for an amazing gem!

I did come across a glitch... I think.

I was testing the rendering system under high stress.

I ran the system in multi threaded mode (running on 16 threads).

While testing, I disabled my own cache management to stress test the rendering engine (rather then the caching system which was storing the rendered string).

I repeatedly asked for the same file to be rendered. 12 threads were doing the asking, 16 threads were doing the rendering, it was all running through Iodine's async IO using HTTP requests, about 200 concurrent requests at a time.

Every time I ran the test, my server would crash with the error:

Assertion failed: (md->work_bufs[BUFFER_SPAN].size == 0), function sd_markdown_render, file markdown.c, line 2898

I'm not complaining about the server crashing, I think it's the right thing to do when experiencing data corruption, but I am worried about the underline cause and the risk of production time failures.

I'm assuming this is multi-threading related, since I didn't get the error in single thread mode.

The following is just my guess (my 2¢) and I hope this helps track down the issue:

At first look, the C code doesn't seem to exit the Global VM Lock, so I thought I might be wrong, however, after a quick read through the code (that's a lot of code and very nicely written, wow), I think the issue is with the markdown struct storing stateful rendering data.

I believe that md->refs and md->work_bufs are susceptible to corruption. It also seems to me that these could be separated from the markdown engine into a separate "state struct", so that multi-threaded rendering with the same markdown render engine wouldn't cause data corruption.

I'm thankful for your time and hope for a fix or a workaround.

Jivanvl pushed a commit to gitlabhq/gitlabhq that referenced this issue Oct 3, 2017

Make Redcarpet Markdown renderer thread-safe
The Redcarpet library is not thread-safe as described in
vmg/redcarpet#570. Since we instantiate
the Redcarpet renderer in a class variable, multiple Sidekiq threads
can access the work buffer and corrupt the state. We work around
this issue by memoizing the renderer on a thread basis.

Closes #36637
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment