<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hi, Sergey,</p>
<p><br>
</p>
<p>thanks for the fixes. Please replace "grop" to "grow" in commit</p>
<p>message. LGTM<br>
</p>
<div class="moz-cite-prefix">On 07.09.2024 08:15, Sergey Kaplun
wrote:<br>
</div>
<blockquote type="cite" cite="mid:Ztvhgw3XohMDy6Su@root">
<pre class="moz-quote-pre" wrap="">Hi, Sergey!
Thanks for the review!
Fixed your comments, rebased the branch on the tarantool/master and
force-pushed it.
On 06.09.24, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Hi, Sergey!
thanks for the patch! LGTM with minor comments below
On 02.09.2024 15:54, Sergey Kaplun wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">From: Mike Pall <mike>
Reported by Sergey Kaplun.
(cherry picked from commit fb22d0f80f291827a4004e16bc589b54bcc4a3c7)
The raising of the OOM error when rehashing the finalizer table (when we
can't allocate a new hash part) leads to crashes in either
`lj_trace_exit()` or `lj_trace_unwind()` due to unprotected error
raising, which either has no DWARF eh_frame or loses the context of the
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I would add a link to a page about eh_frame, for example [1]
Feel free to ignore.
1.
<a class="moz-txt-link-freetext" href="https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html">https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html</a>
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Added, thanks.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">JIT compiler.
This patch drops rehashing of the finalizer table to avoid these
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I would replace "finalizer" with "cdata finalizer".
And I would say about drawbacks of this. Otherwise, it looks like
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Added.
See the new commit message below:
| FFI: Drop finalizer table rehash after GC cycle.
|
| Reported by Sergey Kaplun.
|
| (cherry picked from commit fb22d0f80f291827a4004e16bc589b54bcc4a3c7)
|
| The raising of the OOM error when rehashing the finalizer table (when we
| can't allocate a new hash part) leads to crashes in either
| `lj_trace_exit()` or `lj_trace_unwind()` due to unprotected error
| raising, which either has no DWARF eh_frame [1] or loses the context of
| the JIT compiler.
|
| This patch drops rehashing of the cdata finalizer table to avoid these
| crashes. It will prevent the cdata finalizer table from shrinking when
| the huge amount of the cdata objects is collected by the GC. OTOH, the
| finzlizer table most probably will grop anyway to the old size, so this
| is not crucial.
|
| Sergey Kaplun:
| * added the description and the test for the problem
|
| [1]: <a class="moz-txt-link-freetext" href="https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html">https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html</a>
|
| Part of tarantool/tarantool#10199
| Resolves tarantool/tarantool#10290
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
rehashing was not needed from the beginning.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">crashes.
Sergey Kaplun:
* added the description and the test for the problem
Part of tarantool/tarantool#10199
Resolves tarantool/tarantool#10290
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Usually "Closes" or "Fixes". Feel free to ignore.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
For LuaJIT, we always use "Resolves", since it is __closed__ when the
submodule is bumped in the release branches and master.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">---
Branch:<a class="moz-txt-link-freetext" href="https://github.com/tarantool/luajit/tree/skaplun/lj-1247-fin-tab-rehashing-on-trace">https://github.com/tarantool/luajit/tree/skaplun/lj-1247-fin-tab-rehashing-on-trace</a>
Related Issues:
*<a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/10290">https://github.com/tarantool/tarantool/issues/10290</a>
*<a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1247">https://github.com/LuaJIT/LuaJIT/issues/1247</a>
*<a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/10199">https://github.com/tarantool/tarantool/issues/10199</a>
</pre>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">
<snipped>
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>