<!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>