dynamic_modules: persist module-owned filter state objects across stream recreates

[basundhara-c]

Commit Message: persist module-owned filter state objects across stream recreates
Additional Description:

This adds a dynamic-modules HTTP-filter ABI that lets a module store an opaque, module-owned, non-serializable object in StreamInfo::FilterState at a lifespan that survives recreate_stream, with a module-supplied destructor. It is purely additive to the dynamic-modules extension.

Motivation: A dynamic-module HTTP filter sometimes needs to keep a live, per-stream object graph (channel handles, task handles for an in-flight async exchange) alive across an Envoy recreate_stream(). recreate_stream() destroys the current filter instance and builds a fresh one, so any state the instance held in native memory dies with it. The existing filter-state ABI can't carry it: it's byte-oriented (no way to serialize live handles) and hardcoded to FilterChain lifespan.

This adds two HTTP-filter ABI callbacks plus Rust SDK wrappers:

• set_filter_state_object(key, obj, destructor, life_span): store an opaque, module-owned void* with a lifespan and a module destructor.
• get_filter_state_object(key): borrow it back on any later hook, including the rebuilt instance after a recreate.

Objects stored at Request/Connection lifespan are carried into the new stream by the connection manager; FilterChain objects are not.

The module keys the object under a fixed, well-known name. On a fresh request the key is absent, so the filter creates the object and stores it; after recreate_stream the same key is present on the rebuilt instance, which is the signal to re-attach to the surviving object instead of starting over. The per-stream filter state is the entire handoff ; no request header, random id, or external registry needed.

Risk Level: Low
Testing: Unit and Integration Tests
Docs Changes: N.A
Release Notes: N.A
Platform Specific Features: N.A

[repokitteh-read-only]

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #45888 was opened by basundhara-c.

see: more, trace.

[wbpcode]

From ABI's perspective, looks good to me. Could you take a check also cc @agrawroh

[RyanTheOptimist]

/assign @wbpcode

[agrawroh]

The module destructor runs here with no catch_unwind guard.

The SDK builds with panic = "unwind", and every inbound module callback wraps its body in catch_unwind for exactly this reason. This is the first module-supplied function Envoy stores and calls, and it skips that guard, so a panic in the stored object's drop path is undefined behavior during connection teardown.

Please wrap the destructor call in a catch_unwind trampoline in the SDK, or document that the destructor must never panic and runs on the teardown path.

[agrawroh]

set returns true even when setData did not store. Storing the same key at a different lifespan than a prior entry makes setData hit IS_ENVOY_BUG and return without storing, which is a log-and-continue in release, not an abort.

On that path the wrapper is the only owner, so it is destroyed and the module destructor frees the object, yet the call reports success. The module then holds a freed pointer.

Please detect the non-store and return false. The object is already freed on that path, so the failure contract still holds.

[agrawroh]

here we document a false return "if the key already exists and is marked as read-only", but the setData overload used here has no read-only concept and returns void, so that false can never occur. The only real false today is unavailable stream info.

Please drop the read-only clause and have it describe the real failure modes, including the same-key-different-lifespan conflict. Also, mirror the change in the trait doc in sdk/rust/src/http.rs.

[agrawroh]

The wrapper holds a raw destructor pointer into the module .so but no reference to the config or module, and do_not_close defaults to false.

Connection-lifespan entries can live as long as the connection. This is safe today only because Envoy closes connections before it frees the filter-chain config and unloads the .so, which is an emergent ordering property rather than something this code enforces.

Please make it hold a shared_ptr to the config or module in the wrapper so the .so cannot unload while a live destructor pointer into it exists, or document that Connection lifespan requires do_not_close=true.

[agrawroh]

Can we use IS_ENVOY_BUG here?

[agrawroh]

This hands back a *mut from a &self, and the example mutates through it. The pointer is shared, since a second filter or the rebuilt filter can hold it too.

Could you also document that the module must synchronize any interior mutability and that the pointer is valid only on the owning worker thread?

[agrawroh]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces support for storing and retrieving opaque, module-owned objects in the Envoy filter state for dynamic modules, allowing live objects to survive stream recreation. The feedback identifies critical safety issues: potential null pointer dereferences when accessing stream_info->filterState() in C++ (which could cause crashes), and the need to mark the Rust SDK's set_filter_state_object API as unsafe since it handles raw pointers and destructors.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

The stream_info->filterState() pointer can be null in certain contexts. Dereferencing it directly without a null check will cause a crash. Additionally, if the filter state is not available, we must call the destructor on the module_object to prevent a memory leak, as ownership has already been transferred to Envoy.

bool envoy_dynamic_module_callback_http_set_filter_state_object(
    envoy_dynamic_module_type_http_filter_envoy_ptr filter_envoy_ptr,
    envoy_dynamic_module_type_module_buffer key,
    envoy_dynamic_module_type_filter_state_object_module_ptr module_object,
    envoy_dynamic_module_type_filter_state_object_destructor destructor,
    envoy_dynamic_module_type_filter_state_life_span life_span) {
  auto* filter = static_cast<DynamicModuleHttpFilter*>(filter_envoy_ptr);
  auto* stream_info = filter->streamInfo();
  if (!stream_info) {
    ENVOY_LOG_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::dynamic_modules), debug,
                        "stream info is not available");
    // Ownership transferred to Envoy; free the object rather than leak it.
    if (destructor != nullptr) {
      destructor(module_object);
    }
    return false;
  }
  auto filter_state = stream_info->filterState();
  if (!filter_state) {
    ENVOY_LOG_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::dynamic_modules), debug,
                        "filter state is not available");
    if (destructor != nullptr) {
      destructor(module_object);
    }
    return false;
  }
  absl::string_view key_view(key.ptr, key.length);
  filter_state->setData(
      key_view, std::make_shared<DynamicModuleFilterStateObject>(module_object, destructor),
      toFilterStateLifeSpan(life_span));
  return true;
}

[gemini-code-assist]

The stream_info->filterState() pointer can be null in certain contexts. Dereferencing it directly without a null check will cause a crash.

envoy_dynamic_module_type_filter_state_object_module_ptr
envoy_dynamic_module_callback_http_get_filter_state_object(
    envoy_dynamic_module_type_http_filter_envoy_ptr filter_envoy_ptr,
    envoy_dynamic_module_type_module_buffer key) {
  auto* filter = static_cast<DynamicModuleHttpFilter*>(filter_envoy_ptr);
  auto* stream_info = filter->streamInfo();
  if (!stream_info) {
    ENVOY_LOG_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::dynamic_modules), debug,
                        "stream info is not available");
    return nullptr;
  }
  auto filter_state = stream_info->filterState();
  if (!filter_state) {
    ENVOY_LOG_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::dynamic_modules), debug,
                        "filter state is not available");
    return nullptr;
  }
  absl::string_view key_view(key.ptr, key.length);
  auto* object =
      filter_state->getDataMutable<DynamicModuleFilterStateObject>(key_view);
  if (object == nullptr) {
    return nullptr;
  }
  return object->object();
}

[gemini-code-assist]

In Rust, passing raw pointers and raw function pointers to a safe function is unsafe because it can easily lead to undefined behavior if the pointer or destructor is invalid. The function should be marked unsafe fn to preserve Rust's safety guarantees and require callers to acknowledge the safety invariants.

  /// Store an opaque, module-owned object in the filter state under `key`. Envoy never interprets
  /// the object; it calls `destructor` exactly once when the entry is destroyed. Objects stored at
  /// `Request` or `Connection` lifespan survive `recreate_stream`; `FilterChain` objects do not.
  ///
  /// Typical use: `Box::into_raw(Box::new(state)) as *mut c_void`, with a destructor that calls
  /// `drop(Box::from_raw(ptr as *mut State))`. The module must only recover the object via
  /// [`EnvoyHttpFilter::get_filter_state_object`] and never free it itself.
  ///
  /// Ownership transfers to Envoy on every path: the destructor runs exactly once, either when the
  /// entry is destroyed or before this returns `false`, so a failed store never leaks the object.
  ///
  /// Returns true if the operation is successful, false if the filter state is not accessible or
  /// the key already exists and is read-only.
  ///
  /// # Safety
  ///
  /// The caller must ensure that `object` is a valid pointer and `destructor` is a valid
  /// function pointer that safely deallocates `object`.
  unsafe fn set_filter_state_object(
    &mut self,
    key: &[u8],
    object: *mut c_void,
    destructor: unsafe extern "C" fn(*mut c_void),
    life_span: abi::envoy_dynamic_module_type_filter_state_life_span,
  ) -> bool;

[gemini-code-assist]

Mark the implementation of set_filter_state_object as unsafe fn to match the trait definition, and remove the #[allow(clippy::not_unsafe_ptr_arg_deref)] attribute.

  unsafe fn set_filter_state_object(
    &mut self,
    key: &[u8],
    object: *mut c_void,
    destructor: unsafe extern "C" fn(*mut c_void),
    life_span: abi::envoy_dynamic_module_type_filter_state_life_span,
  ) -> bool {
    abi::envoy_dynamic_module_callback_http_set_filter_state_object(
      self.raw_ptr,
      bytes_to_module_buffer(key),
      object,
      Some(destructor),
      life_span,
    )
  }

[agrawroh]

/wait