<HTML><BODY><div>
<div class="js-helper js-readmsg-msg">
<style type="text/css">
</style>
<div>
<div id="style_15789173131100479537_BODY">
<div class="class_1578941414">
<div>
<div class="js-helper_mailru_css_attribute_postfix js-readmsg-msg_mailru_css_attribute_postfix">
<style type="text/css">
</style>
<div>
<div id="style_15789170601514130393_BODY_mailru_css_attribute_postfix">
<div class="class_1578920217_mailru_css_attribute_postfix">
<div>
<div>Thank you for the review.</div>
<div> </div>
<div>I’ve changed the comment describing box.execute method to explain why an extra flag is needed.</div>
<div>And also added polishing fixes proposed by Igor.</div>
<div> </div>
<div>Result:</div>
<div> </div>
<div>
<div>Using box.execute method before explicitly configuring box<br>
automatically invoked box.cfg nonetheless. Any further calls<br>
to box.cfg caused its reconfiguration which then led to an<br>
error when trying to use execute method again.</div>
<div> </div>
<div>The patch introduces a fix making box.execute immutable.</div>
<div> </div>
<div>Closes #4231<br>
---</div>
<div>Branch:</div>
<div><a href="https://github.com/tarantool/tarantool/compare/eljashm/gh-4231-box.execute-immutable-function">https://github.com/tarantool/tarantool/compare/eljashm/gh-4231-box.execute-immutable-function</a></div>
<div>Issue:</div>
<div><a href="https://github.com/tarantool/tarantool/issues/4231">https://github.com/tarantool/tarantool/issues/4231</a></div>
<div><br>
src/box/lua/load_cfg.lua | 15 ++++++++-<br>
.../gh-4231-immutable-box-execute.test.lua | 32 +++++++++++++++++++<br>
2 files changed, 46 insertions(+), 1 deletion(-)<br>
create mode 100755 test/box-tap/gh-4231-immutable-box-execute.test.lua</div>
<div>diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua<br>
index 85617c8f0..1f9b4392f 100644<br>
--- a/src/box/lua/load_cfg.lua<br>
+++ b/src/box/lua/load_cfg.lua<br>
@@ -517,10 +517,14 @@ setmetatable(box, {<br>
end<br>
})<br>
<br>
+-- Flag needed to keep immutable functions after box reconfiguration<br>
+local box_loaded = false<br>
+<br>
local function load_cfg(cfg)<br>
cfg = upgrade_cfg(cfg, translate_cfg)<br>
cfg = prepare_cfg(cfg, default_cfg, template_cfg, modify_cfg)<br>
apply_default_cfg(cfg, default_cfg);<br>
+ box_loaded = true<br>
-- Save new box.cfg<br>
box.cfg = cfg<br>
if not pcall(private.cfg_check) then<br>
@@ -564,7 +568,16 @@ box.cfg = locked(load_cfg)<br>
-- metatable.<br>
--<br>
function box.execute(...)<br>
- load_cfg()<br>
+ --<br>
+ -- Saving box.execute method before explicit calls to either<br>
+ -- box.execute() or box.cfg{} leads to implicit invocation of<br>
+ -- box.cfg nonetheless. The flag box_loaded makes sure execute<br>
+ -- is an immutable function meaning it won't be overwritten by<br>
+ -- any following attempts to reconfigure box.<br>
+ --<br>
+ if not box_loaded then<br>
+ load_cfg()<br>
+ end<br>
return box.execute(...)<br>
end<br>
<br>
diff --git a/test/box-tap/gh-4231-immutable-box-execute.test.lua b/test/box-tap/gh-4231-immutable-box-execute.test.lua<br>
new file mode 100755<br>
index 000000000..d8940a0b2<br>
--- /dev/null<br>
+++ b/test/box-tap/gh-4231-immutable-box-execute.test.lua<br>
@@ -0,0 +1,32 @@<br>
+#!/usr/bin/env tarantool<br>
+local tap = require('tap')<br>
+local test = tap.test('execute')<br>
+test:plan(4)<br>
+<br>
+--<br>
+-- gh-4231: box.execute should be immutable function meaning it's<br>
+-- address doesn't change after first box.cfg implicit invocation<br>
+--<br>
+<br>
+local function execute_is_immutable(execute, cmd, msg)<br>
+ local status, err = pcall(function()<br>
+ execute(cmd)<br>
+ end)<br>
+ test:ok(status and err == nil, msg)<br>
+end<br>
+<br>
+local box_execute_stub = box.execute<br>
+test:is(box_execute_stub, box.execute, "execute is not the same before box.cfg")<br>
+execute_is_immutable(box_execute_stub, "SELECT 1",<br>
+ "execute does not work properly before box.cfg")<br>
+<br>
+local box_execute_actual = box.execute<br>
+-- explicit call to load_cfg<br>
+box.cfg{}<br>
+-- checking the function was not reconfigured, i.e. adress stays the same<br>
+test:is(box_execute_actual, box.execute, "execute is not the same after box.cfg")<br>
+execute_is_immutable(box_execute_actual,<br>
+ "CREATE TABLE t1 (s1 INTEGER, PRIMARY KEY (s1));",<br>
+ "execute does not work properly after box.cfg")<br>
+<br>
+os.exit(test:check() and 0 or 1)<br>
-- <br>
2.24.0</div>
</div>
<div class="mail-quote-collapse">
<blockquote style="border-left:1px solid #0857A6;margin:10px;padding:0 0 0 10px;"><span data-email="alexander.turenko@tarantool.org" data-name="Alexander Turenko" data-quote-id="1695349805772874934" data-timestamp="1577369280" data-type="sender"><span>Четверг, 26 декабря 2019, 17:08 +03:00 от Alexander Turenko <<a rel="noopener noreferrer">alexander.turenko@tarantool.org</a>>:<br>
</span> </span>
<div data-quote-id="1695349805772874934" data-type="body">
<div>
<div id="">
<div class="js-helper_mailru_css_attribute_postfix_mailru_css_attribute_postfix js-readmsg-msg_mailru_css_attribute_postfix_mailru_css_attribute_postfix">
<style type="text/css">
</style>
<div>
<div id="style_15773693150941174231_BODY_mailru_css_attribute_postfix_mailru_css_attribute_postfix">> function box.execute(...)
<div class="mail-quote-collapse">> - load_cfg()<br>
> + --<br>
> + -- Calling box.execute before explicitly configuring box led to<br>
> + -- automatic box.cfg invocation nonetheless. Following explicit<br>
> + -- attempts to configure box led to its reconfiguratin and as a <br>
> + -- result - to an error when trying to use execute method again. <br>
> + -- The check makes sure box.execute is an immutable function.<br>
> + --</div>
<br>
I would describe all this hell with registering lbox_execute as<br>
box.execute, moving it to box_cfg_guard_whitelist, adding this<br>
box.execute function, then replacing back with lbox_execute at<br>
load_cfg().<br>
<br>
This description would make clear why everything is fine if we just call<br>
box.execute(), but extra flag is necessary if a user saved box.execute<br>
before first box.execute() or box.cfg() call.</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<div> </div>
<div data-signature-widget="container">
<div data-signature-widget="content">
<div>--<br>
Maria Khaydich</div>
</div>
</div>
<div> </div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</BODY></HTML>