mlevkov opened a new issue, #4100:
URL: https://github.com/apache/iggy/issues/4100
## What happens
`SourceContainer::open` in `core/connectors/sdk/src/source.rs` stores the
source before it knows
whether opening worked, and returns a failure code afterwards:
```rust
let mut source = factory(id, config, state);
let runtime = get_runtime();
let result = runtime.block_on(source.open());
self.id = id;
self.source = Some(Arc::new(source)); // unconditional
if result.is_ok() { 0 } else { 1 }
```
The `source_connector!` macro then inserts into `INSTANCES` regardless of
that code:
```rust
let result = instance.open(/* ... */);
INSTANCES.insert(id, container); // unconditional
result
```
So a plugin whose `open()` fails leaves a live `INSTANCES` entry under its
`plugin_id`, holding the
constructed `Source`.
Nothing on the runtime side closes it. `init_source` returns `Err`, and the
boot path records the
plugin with `error` set and moves on, while the restart path returns through
`?`. The guard added in
#4064 is armed only after `init_source` succeeds, so it does not cover this
case by design.
## Why it matters, and where it does not
For a plugin whose `open()` only allocates, this is wasted memory until the
process exits.
It becomes a live fault for a plugin that acquires a process-global resource
and can then fail
*after* acquiring it, since nothing will ever release it.
Worth stating plainly, because it came up while reviewing #3798: the HTTP
source is **not** such a
plugin. Its `open()` calls `server::join` as the last fallible step and does
nothing fallible after,
and every `join` failure path returns before the instance reaches the
published route table. A
failed `open()` there leaves an inert object, not a joined listener. This
issue is about the general
case.
## Fix
Skip the insert and drop the container when `open()` returned non-zero.
Not by closing from the runtime side: the SDK has already stored the source,
so a close would run
`Source::close()` on an instance whose `open()` never completed, which
plugins are not written to
expect.
## Provenance
Raised by @hubcio in review on #4064, out of that PR's diff and explicitly
non-blocking for it, with
the fix above. Filed separately so it does not disappear when #4064 merges.
### Contribution
- [x] I'm willing to submit a pull request to fix this bug
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
