{"action":"opened","issue":{"url":"https://api.github.com/repos/schneems/maildown/issues/40","repository_url":"https://api.github.com/repos/schneems/maildown","labels_url":"https://api.github.com/repos/schneems/maildown/issues/40/labels{/name}","comments_url":"https://api.github.com/repos/schneems/maildown/issues/40/comments","events_url":"https://api.github.com/repos/schneems/maildown/issues/40/events","html_url":"https://github.com/schneems/maildown/issues/40","id":309046308,"number":40,"title":"Possible race condition in the Maildown::MarkdownEngine.default_html_block method","user":{"login":"yuki24","id":386234,"avatar_url":"https://avatars0.githubusercontent.com/u/386234?v=4","gravatar_id":"","url":"https://api.github.com/users/yuki24","html_url":"https://github.com/yuki24","followers_url":"https://api.github.com/users/yuki24/followers","following_url":"https://api.github.com/users/yuki24/following{/other_user}","gists_url":"https://api.github.com/users/yuki24/gists{/gist_id}","starred_url":"https://api.github.com/users/yuki24/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yuki24/subscriptions","organizations_url":"https://api.github.com/users/yuki24/orgs","repos_url":"https://api.github.com/users/yuki24/repos","events_url":"https://api.github.com/users/yuki24/events{/privacy}","received_events_url":"https://api.github.com/users/yuki24/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-03-27T16:38:42Z","updated_at":"2018-03-27T16:38:42Z","closed_at":null,"author_association":"NONE","body":"I was wondering how https://github.com/yuki24/did_you_mean/issues/102 could occur. One thing I noticed is that in the maildown gem, the `kramdown` gem is lazily loaded on the fly, which adds a handful of constants dynamically. In an extreme condition, this may cause a race condition. Consider the following scenario where a Sidekiq instance runs two threads (workders) that attempt to run an email delivery job:\r\n\r\nThread 1 | Thread 2\r\n---------|---------\r\n| calls `MarkdownEngine.default_html_block` | - |\r\n| - | calls `MarkdownEngine.default_html_block` |\r\n| [attempts to `require 'kramdown'`](https://github.com/schneems/maildown/blob/2ea4c8e7a1b3bcf7b84d3f43405ed0ae5e4c5be5/lib/maildown/markdown_engine.rb#L52) for the first time | - |\r\n| evaluates [the line that adds a `Kramdown` constant](https://github.com/gettalong/kramdown/blob/84102c8b654a54eb8307fe7703b1414b537eb016/lib/kramdown/document.rb#L46) but [doesn't reach the line 74 that adds `Kramdown::Document`](https://github.com/gettalong/kramdown/blob/REL_1_16_2/lib/kramdown/document.rb#L74) | - |\r\n| - | [evaluates `unless defined? Kramdown`](https://github.com/schneems/maildown/blob/2ea4c8e7a1b3bcf7b84d3f43405ed0ae5e4c5be5/lib/maildown/markdown_engine.rb#L52) (now evaluates to `unless true`) |\r\n| - | attempts to call [`html_block.call` on the proc that has a reference to the `Kramdown::Document`](https://github.com/schneems/maildown/blob/2ea4c8e7a1b3bcf7b84d3f43405ed0ae5e4c5be5/lib/maildown/markdown_engine.rb#L24) that does not exist yet |\r\n\r\nIn this scenario, the thread 2 would raise a `NameError` because the `Kramdown` constant is loaded but the `Kramdown::Document` is not. Also, it would only occur once after a cold reboot since the constant would be available once it's loaded by the thread 1.\r\n\r\nI wasn't sure what fixes you'd prefer, so just wanted to bring this up. Thanks!"}}
{"actor":{"display_login":"yuki24"}}