wasm: clean up the code #36556
wasm: clean up the code #36556
Conversation
Signed-off-by: wangbaiping <wangbaiping@bytedance.com>
Signed-off-by: wangbaiping <wangbaiping@bytedance.com>
Signed-off-by: wangbaiping <wangbaiping@bytedance.com>
Signed-off-by: wangbaiping <wangbaiping@bytedance.com>
|
|
||
| plugin_ = std::make_shared<Plugin>(config, direction, context.localInfo(), metadata); | ||
|
|
||
| auto callback = [this, &context](WasmHandleSharedPtr base_wasm) { |
There was a problem hiding this comment.
nit: maybe put ASSERT_MAIN_OR_TEST_THREAD?
| init_manager, context.mainThreadDispatcher(), context.api(), | ||
| context.lifecycleNotifier(), remote_data_provider_, | ||
| std::move(callback))) { | ||
| throw Common::Wasm::WasmException( |
There was a problem hiding this comment.
Can we restructure this to avoid exceptions?
There was a problem hiding this comment.
I finally add a TODO first. Because this is in the constructor and we need to use creation_status to handle the error. That means we need to change code of lots of different positions at where this PluginConfig is used.
We can put it to separated PR.
|
|
||
| if (is_singleton_handle_) { | ||
| // Use critical log because this error should not happen in production. | ||
| ENVOY_LOG(critical, "CreateContext() only works for thread local plugins."); |
There was a problem hiding this comment.
why not release assert if this never should be called by the implementation?
There was a problem hiding this comment.
This check is finally removed at #36456. Finally, we use the same way to treat the singleton handle and thread local handle.
So, I think we can leave it there for now?
| return singleton_handle_ != nullptr ? singleton_handle_->wasmOfHandle() : nullptr; | ||
| } | ||
|
|
||
| ASSERT(thread_local_handle_ != nullptr); |
There was a problem hiding this comment.
RELEASE_ASSERT since it would crash anyways?
There was a problem hiding this comment.
From the current code base, seems we prefer the ASSERT even in this type case? Let's me know if you think this is a block point.
Signed-off-by: wangbaiping <wangbaiping@bytedance.com>
Commit Message: wasm: clean up the code
Additional Description:
When I doing the #36456, I found there are lots of redundant code in the wasm extensions. And the wasm loading and creations are spread out in multiple different positions.
This redundancy and fragmentation make #36456 become more and more complex.
Finally, I split the code clean up out as an independent PR.
This PR doesn't change any logic but only merge duplicated logic.
Risk Level: n/a.
Testing: n/a.
Docs Changes: n/a.
Release Notes: n/a.