libplugin: use json stream helpers and add sanity tests #3480
Conversation
darosior
force-pushed
the
libplugin_json_stream
branch
from
5e8e9ca
to
f39e40f
Compare
|
Rebased after #3443 rebase. |
darosior
force-pushed
the
libplugin_json_stream
branch
6 times, most recently
from
fc74bef
to
66c0ae3
Compare
|
Re-rebased :) |
darosior
force-pushed
the
libplugin_json_stream
branch
from
66c0ae3
to
10dfa1f
Compare
darosior
force-pushed
the
libplugin_json_stream
branch
from
10dfa1f
to
ede4582
Compare
|
I'm not really happy with the read after free fix, if there are suggestions.. |
The "technically correct" answer is to create the infrastructure to unregister options in ccan/opt. To do anything else risks duplicate options in the option table as we remove then re-add a plugin. Let me create that now... |
darosior
force-pushed
the
libplugin_json_stream
branch
3 times, most recently
from
b5f41a9
to
9746a94
Compare
The usual json_stream starters and command_result enders.
This adds helpers to start and send a jsonrpc request using json_stream in order to benefit from the helpers. This then simplifies existing plugins RPC requests by using json_stream helpers.
We mark the test as xfail() as it exposes that libplugin's PLUGIN_RESTARTABLE was not taken into account !
As a separated commit because it was pre-existent (changelog + xfail test). This also fix a logical problem in lightningd/plugin_control: we were assuming a plugin started with 'plugin start' but which did not comport a 'dynamic' entry in its manifest to be dynamic, though it should have been treated as static. Changelog-fixed: plugins: Dynamic C plugins can now be managed when lightningd is up
Co-authored-by: Rusty Russell <[email protected]>
darosior
force-pushed
the
libplugin_json_stream
branch
from
9746a94
to
45a06db
Compare
This also remove the now duplicate plugin_hook_unregister_all(), added in the tal destructor of the struct plugin.
darosior
force-pushed
the
libplugin_json_stream
branch
from
45a06db
to
77e7f0f
Compare
| * This undoes opt_register[_early]_[no]arg. Returns true if the option was | ||
| * found, otherwise false. | ||
| */ | ||
| bool opt_unregister(const char *names); |
There was a problem hiding this comment.
BTW, CCAN updates should be done via "make update-ccan" (optional: CCAN_NEW="". Assumes that ccan you want is in ../ccan.
Easy to fix this post, however.
There was a problem hiding this comment.
I did it this way then withdrawn it because of the huge diff, will fix!
| @@ -791,6 +791,7 @@ def test_rpc_command_hook(node_factory): | |||
| l1.rpc.plugin_stop('rpc_command.py') | |||
|
|
|||
|
|
|||
| @flaky | |||
There was a problem hiding this comment.
Why is this now @flaky?
There was a problem hiding this comment.
Because it started failing randomly on Travis but I was not able to reproduce locally..
Btw, i can no longer restart Travis jobs.. Don't know if it's a perm error or anything
This is a follow-up to #3443 (based on it and implements the first of the left TODOs).
This moves all plugins from "raw"
jouts to using higher-leveljson_streamhelpers.This also adds sanity functional tests for libplugin plugins, which uncovered a bug.