Andrew Hilger
90d · built 2026-08-09
90-day totals
- Commits
- 119
- Grow
- 5.5
- Maintenance
- 10.1
- Fixes
- 2.8
- Total ETV
- 18.4
30-day trajectory
Last 30 days vs. the 30 days before. Up arrows on Growth and ETV mean improvement; up arrow on Fixes share means more time on fixes (worse).
↓-75.4 %
vs 57 prior
↓-22.5 pp
recent vs prior
↑+53.4 pp
recent vs prior
Daily performance
Daily ETV, stacked by Growth, Maintenance and Fixes.
Work-mix over time
Share of Growth / Maintenance / Fixes over a rolling 7-day window. Reads as 'where is effort flowing right now'.
Repository spread
Where this developer's commits land. Concentrated work (top1 > 80%) vs polymath spread (top1 < 30%).
Most impactful commits
Top 20 by ETV in the 90-day window.
- 1.4ETVAdd pure Python reflection library for thrift-python Summary: Add a pure Python reflection library for thrift-python in `thrift/lib/python/reflection/`, providing a superset of the py3 reflection API. `constants_reflection.py` — immutable, hashable spec classes for representing Thrift constant values: - ThriftType enum covering all Thrift types (bool through void) - ConstantSpec, ConstantStructSpec, ConstantUnionSpec, ConstantEnumSpec, ConstantListSpec, ConstantSetSpec, ConstantMapSpec - All types frozen after __init__ via _ImmutableMeta metaclass `types_reflection.py` — type introspection specs: - FieldSpec, StructSpec, ListSpec, SetSpec, MapSpec (all immutable) - `.kind` properties mapping ThriftType to py3-compatible NumberType - `.annotations` computed properties for backward-compatible annotation access - `__iter__` on all spec classes matching py3's yield order for drop-in compatibility - Thread-safe cached `inspect()`/`inspectable()` entry points No thrift compiler changes; generated code support follows in D101724784. Reviewed By: createdbysk Differential Revision: D101724783 fbshipit-source-id: a2515fdb86c3ff0c875e481f0aaf27cd498383c9github.com-facebook-fbthrift · a2ecf8d0 · 2026-05-14
- 1.3ETVServer-side interactions: single request/response runtime Summary: Groundwork landed in D107442265 (typed `FunctionEntry`). This adds the thrift-python server-side interaction runtime for single-request/single-response and oneway methods, with no compiler support yet (D107456114 adds codegen). Validated end-to-end with hand-written interaction handlers driven by a real thrift-python client over Rocket. - `FunctionEntry` interaction routing fields: `interaction` (the interaction name, set on both the factory and the inside-interaction entries), `creates_interaction` (True for the factory that creates the interaction; False for inside methods and ordinary methods), and `interaction_factory` (the per-session `create<Interaction>` constructor). `HandlerFunc`/`PythonMetadata` thread this into the framework. `makeHandlerFunc` builds ordinary entries; `makeInteractionHandlerFunc` builds factory/inside entries and derives `interactionType` from `createsInteraction`. - `PythonTile : apache::thrift::Tile` holds the per-session Python handler. It is constructed with the GIL already held (both sites build it inside a `PyGILState_Ensure` region or on the Python executor), so the constructor only `Py_XINCREF`s. The destructor schedules the `Py_DECREF` on the Python (asyncio-loop) executor via a `folly::Executor::KeepAlive` instead of synchronously acquiring the GIL on an arbitrary teardown thread (unsafe during interpreter finalization). - `createInteractionImpl` (implicit `performs`/lazy creation) and `maybeFulfillTilePromise` (explicit factory) construct the tile from the user`s factory. Inside-interaction dispatch keeps the *unbound* dispatch function on the entry and binds it to the per-session handler in Cython (`func.__get__(handler)`) -- getattr-free, no `PyMethod_New` in C++. - Error handling: a user factory exception is no longer swallowed. `callInteractionFactory` propagates it per the Python C-API convention (nullptr + error set); `createInteractionImpl` surfaces it via `folly::python::handlePythonError` (framework reports it as an interaction-constructor error), and `maybeFulfillTilePromise` returns it (`folly::exception_wrapper`) so the dispatch fails the factory request with the real message rather than stranding the callback. The `createInteractionImpl` GIL region is exception-safe (`SCOPE_EXIT`). Reviewed By: prakashgayasen Differential Revision: D107452899 fbshipit-source-id: bbd9cf1afb7934289b47f6875cadf03ae666b8f5github.com-facebook-fbthrift · 669d5688 · 2026-06-10
- 0.8ETVgenerate services reflection from thrift compiler Summary: Wire up the thrift compiler to generate `thrift_services_reflection.py` for each thrift module that has services. This enables `inspect()` on service interfaces and clients, returning a `ServiceSpec` with full function, argument, exception, and annotation information. **Compiler changes:** - New whisker template `thrift_services_reflection.py.whisker` that generates `get_reflection__<ServiceName>()` functions, mirroring the pattern from `thrift_reflection.py.whisker` for structs - Add `generate_services_reflection()` to `t_mstch_python_generator.cc` - Add `__get_reflection__` to generated service interfaces (`services.whisker`) and clients (`client_body.whisker`) **Build system:** - Register `thrift_services_reflection.py` in `python.bzl` so the generated file is included in the `-services` Buck target **Tests:** - New `test/services_reflection.py` with end-to-end tests using `TestingService` from `test_thrift.thrift` - Tests cover: service name, function specs, void/oneway functions, arguments with various types, exceptions, parent service inheritance, structured annotations, immutability, and client/service parity Reviewed By: prakashgayasen Differential Revision: D105987688 fbshipit-source-id: f18060b2d945014b0f04fa9df100d9d5a29bcb23github.com-facebook-fbthrift · c49f4d4d · 2026-05-26
- 0.6ETVDrop the isset byte array for isset-disabled immutable structs Summary: Immutable thrift-python structs store their fields in a `_fbthrift_data` tuple that, until now, always reserved element 0 for an isset `bytes` array (so field `i` lived at index `i + 1`). That isset array costs `33 + numFields` bytes per struct and is only ever read by `isset_DEPRECATED()`, which is enabled for a tiny minority of structs (those compiled with `enable_isset_deprecated_unsafe`). Every other struct paid the memory and the per-field isset writes on deserialization for nothing. This makes the data-holder layout per-type: - **Default (isset-disabled, the common case):** the tuple has exactly `numFields` elements with no isset header; field `i` lives at index `i`. No `bytes` object is allocated, and deserialization no longer writes per-field isset flags. - **Isset-deprecated (rare):** unchanged `numFields + 1` layout with the isset array at element 0. The layout is chosen once at class-creation time from `enable_isset_deprecated`, so the (de)serialize hot path gains no per-field branch: the per-type `StructInfo` carries the right `memberOffset`, `getIsset`/`setIsset` function pointers, and allocation helper. A new `TupleContainer` policy (no header, `kFieldStartIndex = 0`) sits alongside the existing one, now named `IssetDeprecatedTupleContainer` (`kFieldStartIndex = 1`); the C++ entry points dispatch on `immutableStructInfoHasIsset`. On the Cython side, `StructInfo.field_start_index` is precomputed so field access (`_fbthrift_py_value_from_internal_data`, `_StructPrimitiveField`) and `set_struct_field` are branch-free, and `__call__` copies field values over their tuple indices directly. The capi (C-API) marshalling generator is updated to match: `t_mstch_python_capi_generator` emits 0-based `tuple_pos` arrays for isset-disabled structs (1-based only when isset is enabled), and the constructor template selects the matching runtime allocator (`createStructTuple` vs `createStructTupleWithDeprecatedIsset`) and field setter (`setStructField` vs `setStructFieldIssetDeprecated`). capi fixtures are regenerated. Exceptions (`GeneratedError`) never enable isset, so they always use the default no-header layout. Naming convention: the plain names are the default no-isset path (`TupleContainer`, `createStructTuple`, `getImmutableIsset`, `setStructField`); the deprecated isset path is uniformly suffixed (`IssetDeprecatedTupleContainer`, `createStructTupleWithDeprecatedIsset`, `getImmutableIssetDeprecated`, `setStructFieldIssetDeprecated`, `setStructIssetDeprecated`, `issetDeprecatedEnabled_`). Reviewed By: prakashgayasen Differential Revision: D109898403 fbshipit-source-id: e0fccd27cd38c2db9a64342a65565f893fc646c8github.com-facebook-fbthrift · 99c6fda8 · 2026-06-30
- 0.6ETVAdd thrift-python reflection code generation Summary: Add thrift compiler codegen for thrift-python reflection. Every thrift-python module now generates a `thrift_reflection.py` file containing `StructSpec`/`FieldSpec` definitions for each struct, union, and exception. Generated immutable types get a `__get_reflection__` staticmethod that lazily imports and returns the reflection spec. Compiler changes: - New whisker template `thrift_reflection.py.whisker` generating `StructSpec`/`FieldSpec` with field id, name, `py_name`, type, `ThriftType`, `Qualifier`, default values, and structured annotations - New `reflection/const_value.whisker` and `reflection/const_struct.whisker` partials for rendering annotation constant values with correct `ThriftType` precision (distinguishes `BYTE`/`I16`/`I32`/`I64` for integers, `FLOAT`/`DOUBLE` for floating point, `SET`/`LIST` for sequences) - `const_struct.whisker` emits the actual annotation type class for `ConstantStructSpec.struct_type` (not a placeholder), using inline imports for annotation type modules - `types.whisker` updated to emit `__get_reflection__` on all generated struct/union/exception classes - `t_mstch_python_generator.cc` updated to generate the reflection module - `python.bzl` updated to include `thrift_reflection` in the build rule Runtime changes: - `container_typedefs.py` gains `__get_reflection__` methods on `_ListTypedefBase`, `_SetTypedefBase`, `_MapTypedefBase` that derive `ListSpec`/`SetSpec`/`MapSpec` from typeinfo - End-to-end integration tests in `reflection.py` exercising codegen with real generated types from `test_thrift.thrift` Reviewed By: createdbysk Differential Revision: D101724784 fbshipit-source-id: 6060e8ebab79da1459b7988f73f6e31ff2d89d04github.com-facebook-fbthrift · d44a557b · 2026-05-15
- 0.6ETVserver: interaction factory with initial response Summary: Adds thrift-python server-side support (and test coverage) for an interaction factory that returns an initial response alongside the new interaction, i.e. `Interaction, Response factory()` whose handler returns `tuple[Interaction, Response]` (the TileAndResponse shape). Previously the server runtime could only create an interaction Tile via the zero-arg `create<Interaction>()` factory, which never sees the factory arguments, so a factory whose per-session state derives from its arguments could not be expressed. Codegen (thrift-python `services` templates): a factory-with-initial-response handler is typed `-> Tuple[<Interaction>Interface, <Response>]`; the generated wire wrapper unpacks `(tile, value)`, installs the returned Tile via `install_interaction_tile(tile)`, and returns the serialized `value` through the ordinary single-response path. The `FunctionEntry` is flagged `returns_initial_response=True`. Runtime: `FunctionEntry.returns_initial_response` flows to C++ `HandlerFunc.returnsInitialResponse`, and `prepareInteractionDispatch` skips the zero-arg pre-create for such methods (leaving the `TilePromise` parked). The wrapper installs the handler-returned instance directly from Python via a thin helper, `install_interaction_tile` -> C++ `installInteractionTileFromHandler`, which derives the connection's EventBase from the request context's transport, wraps the instance in a `PythonTile`, and fulfills the parked `TilePromise` on the EventBase thread. The ordinary req/resp dispatch path (promises, `serverCallback_coro`) is unchanged. Because the install call sits after the handler invocation inside the wrapper's declared-exception `try`, error propagation falls out for free: a `.thrift`-declared exception raised by the factory surfaces to the client as that exception, an undeclared exception is translated to `ApplicationError(UNKNOWN)`, and in neither case is a Tile installed. IDL/handler/test (interaction test `calculator.thrift`): `Counter, CounterSnapshot initializedCounter(1: i32 start) throws (1: NegativeError err)` + `CounterSnapshot`; `CalculatorHandler.initializedCounter` builds the Tile from `start` and returns `(CounterHandler, CounterSnapshot)`. Tests: - `test_handshake_factory_with_initial_response`, `test_factory_with_initial_response_isolation` -- happy path and per-session isolation. - `test_factory_with_initial_response_declared_exception` -- declared `NegativeError` from the factory surfaces as-is. - `test_factory_with_initial_response_unexpected_exception` -- undeclared `RuntimeError` becomes `ApplicationError(UNKNOWN)`. Note: separated out from an unrelated thrift-python patch change that was present in the same working copy. landed-with-radar-review Reviewed By: prakashgayasen Differential Revision: D108376358 fbshipit-source-id: 11207bfe04aa7ce5ccb997e0a7bce7a995b49ec0github.com-facebook-fbthrift · 0f0af687 · 2026-06-15
- 0.5ETVadd stream, sink, and bidi reflection to services reflection Summary: Add `StreamSpec` and `SinkSpec` classes to the services reflection API, and extend `FunctionSpec` with optional `stream` and `sink` fields to capture streaming function metadata that was previously lost. **Runtime (`services_reflection.py`):** - `StreamSpec`: `elem_type`, `elem_thrift_type`, `exceptions` (stream-level exceptions) - `SinkSpec`: `payload_type`/`payload_thrift_type`, `payload_exceptions`, `final_response_type`/`final_response_thrift_type`, `final_response_exceptions` - `FunctionSpec.stream` / `FunctionSpec.sink`: `None` for regular functions; for bidi, both are set (sink has no `final_response_type`) - `FunctionSpec.return_type` / `FunctionSpec.return_thrift_type`: for streaming functions, these represent the *initial response* only (None/VOID when there is no initial response). Stream/sink element types are accessed via the `stream` and `sink` fields. - `FunctionSpec.is_streaming` / `FunctionSpec.is_bidi` convenience properties **Compiler template (`thrift_services_reflection.py.whisker`):** - Emit `stream=_StreamSpec(...)` for server-stream functions - Emit `sink=_SinkSpec(...)` for client-sink functions - Emit both for bidirectional stream functions (bidi sink omits `final_response_type`) - Handle stream/sink-level exceptions and non-typedef container element types Reviewed By: prakashgayasen Differential Revision: D106138990 fbshipit-source-id: f9a494fb54e67c1bb5fad2b7aab116511e46d525github.com-facebook-fbthrift · b0fb0e57 · 2026-07-08
- 0.5ETVFix OmniClient teardown use-after-free on a freed EventBase Summary: `apache::thrift::python::client::OmniClient::~OmniClient()` performed a heap-use-after-free on a `folly::EventBase` at process/interpreter shutdown, SIGSEGV-ing (exit 139) *after* the process had finished its work. Root cause: a thrift-python sync client binds its channel to an IO executor's `EventBase`. That `EventBase` can be destroyed before the client at shutdown -- either when the global IO executor singleton is torn down (the sync client uses `folly::getGlobalIOExecutor()->getEventBase()`), or when an `IOThreadPoolExecutor`'s own worker-thread teardown frees the `EventBase` while the executor is otherwise still returnable. The destructor grabbed a keep-alive token only at destruction time: ``` auto* eb = channel_->getEventBase(); if (eb != nullptr) { folly::getKeepAliveToken(eb).add([channel = std::move(channel_)](auto&&) {}); } ``` so `folly::getKeepAliveToken(eb)` -- whose `keepAliveAcquire()` dereferences `eb` -- ran on an already-freed `EventBase` -> UAF. This surfaced while migrating `monitoring/obc/py/OBCClient.py` to thrift-python, breaking the `instagram.distillery.opt` fbpkg build. Fix: arm a `folly::EventBase::OnDestructionCallback` death probe on the channel's `EventBase` at construction. In `~OmniClient()`, `cancel()` reports whether the `EventBase` outlived the client without dereferencing the possibly-freed pointer -- it returns false only if `~EventBase` already ran the probe. If the `EventBase` is gone, the channel can neither be used nor destroyed on it, so we intentionally leak it (`folly::annotate_object_leaked`) -- harmless during process shutdown and far preferable to a UAF. Otherwise the `EventBase` is alive and the channel is destroyed on its thread as before. Two simpler approaches do not work. A lifetime `KeepAlive<EventBase>` would hang teardown: `~EventBase` spins `while (loopKeepAliveCount() > 0)`, so holding a token across shutdown blocks the global IO executor teardown instead of crashing. Pinning the global IO executor across the hop is insufficient: it can stay returnable (does not throw) while the `EventBase` behind it has already been freed by worker-thread teardown, so the pin neither detects nor prevents the UAF. The identical idiom in `OmniInteractionClient::terminate()` is left as-is: `InteractionId::~InteractionId()` has a fatal `CHECK_EQ(id_, 0)` and `release()` is private, so a dead-executor interaction can only be handled by terminating while the executor is alive (a separate contract issue). Repro and root-causing based on D113108222 (mogre). Reviewed By: prakashgayasen Differential Revision: D113127237 fbshipit-source-id: bd96b0118db59056372bcec2fa44815649e27384github.com-facebook-fbthrift · cc875c7a · 2026-07-23
- 0.5ETVAdd runtime reflection support for container typedef types Summary: Add runtime reflection support for container typedef types and Cython container instances. Runtime changes: - `container_typedefs.py`: `_ListTypedefBase`, `_SetTypedefBase`, `_MapTypedefBase` gain `__get_reflection__` classmethods that derive `ListSpec`/`SetSpec`/`MapSpec` from typeinfo - `types.pyx`: Cython `List`, `Set`, `Map` classes gain `__get_reflection__` instance methods that extract type info from the internal `_fbthrift_val_info`/`_fbthrift_key_info` cdef attributes, enabling `inspect(easy().val_list)` on field values - `types_reflection.py`: `inspect()` handles container instances (dispatching to `__get_reflection__`) before the class-based path; `inspectable()` returns `True` for container instances; `overload` signatures provide precise return types (`ListSpec`, `SetSpec`, `MapSpec`, `StructSpec`) based on input type - Moved `typedef_compatibility.py` and new `reflection_compatibility.py` into `test/compatibility/` with their own BUCK file - `thrift_reflection.py.whisker`: guard `import thrift.python.container_typedefs` inside `try/except ImportError` for backward compatibility with older runtimes Test coverage: - `InspectContainerTypedefTest`: `inspect(I32List)`, `inspect(SetI32)`, `inspect(StrI32ListMap)` with full spec assertions including nested `inspect(spec.value)` on map values - `inspect(I32List([1, 2, 3]))` and `inspect(easy().val_list)` for instance-based inspection - `inspectable()` on container classes and instances - `ReflectionCompatibilityTest`: verifies `__get_reflection__()` and `inspect()` return `None` when `thrift.python.reflection` and `thrift.python.container_typedefs` are blocked Reviewed By: prakashgayasen Differential Revision: D101724785 fbshipit-source-id: b4da857a7c5fc88d9a489d25443f943ffc738bfegithub.com-facebook-fbthrift · 6c29aed4 · 2026-05-19
- 0.5ETVEmit field default values in thrift-python reflection codegen Summary: `FieldSpec.default` now contains a correctly-typed Python value instead of always being `None`. For optional fields, `default` is `None`. For unqualified fields, `default` is a value whose Python type matches `FieldSpec.type`: the explicit IDL default if one exists, or the intrinsic zero value (`0`, `""`, `False`, `0.0`, `b""`, `EnumClass(0)`, `StructClass()`, `ContainerClass()`) otherwise. Template changes: - `thrift_reflection.py.whisker`: inline default logic distinguishing optional (None), explicit default (via `reflection/default_value` partial), and intrinsic default (type-specific zero values using `python_type_class`/`field_container_var`) - New `reflection/default_value.whisker`: renders explicit defaults as raw Python values, including struct constructor calls with kwargs and container literals. The template is recursive — it calls itself for nested struct fields, map keys/values, and list/set elements. Test changes: - Added `_get_field_spec(spec, field_name)` helper to replace repeated `fields_by_name` dict-comprehension pattern across all test classes - Added `NestedDefaultStruct` to `test_thrift.thrift` with fields exercising recursive default rendering: struct-inside-struct (`Messy` with nested `Runtime`), `list<SimpleStruct>` with non-empty default, `map<string, SimpleStruct>` with non-empty default - Strengthened `test_struct_default` to assert full equality against the expected `Runtime` value (including nested enum and list fields) Reviewed By: prakashgayasen Differential Revision: D103942994 fbshipit-source-id: 77bfff9612432eed4d5c12a99ac8436ff83b61f9github.com-facebook-fbthrift · c7dbf4f5 · 2026-05-20
- 0.4ETVFix OmniClient send-path use-after-free on a freed EventBase Summary: `apache::thrift::python::client::OmniClient` binds its channel to an IO executor's `EventBase`. At process/interpreter shutdown that `EventBase` can be destroyed before the client -- either when the global IO executor singleton is torn down (`folly::getGlobalIOExecutor()`, freed by `SingletonVault::destroyInstances()` during `Py_FinalizeEx`), or when an `IOThreadPoolExecutor`'s worker-thread teardown frees the `EventBase` while the executor is otherwise still returnable. D113127237 fixed the *teardown* flavor of this bug (the `~OmniClient` destructor). This diff fixes the *send* flavor: a `sync_send`/`oneway_send` issued after the `EventBase` is gone reaches `OmniClient::sendImpl` -> `RequestChannel::sendRequestAsync`, which hops onto the freed `EventBase` via `EventBase::runInEventBaseThread` -> heap-use-after-free -> SIGSEGV. In prod this crashed on a Python worker thread during shutdown, with a stack through `OmniClient::sync_send` -> `semifuture_send` -> `sendImpl` -> `sendRequestAsync` -> `folly::EventBase::runInEventBaseThread`. Once the `EventBase` (and its IO thread) is destroyed there is no live transport to carry the request, so the request cannot be sent. The fix fails fast and cleanly instead of dereferencing freed memory: - `EventBaseLivenessProbe::onEventBaseDestruction` now flips an atomic `destroyed` flag (the flag lives inside the probe so it moves with it in the move constructor). - `sendImpl` checks that flag before dispatching and, if the `EventBase` is gone, throws `TTransportException(NOT_OPEN)`. On the response path this flows through the existing `semifuture_send` catch -> `resp.buf` error -> `create_py_exception`, surfacing in Python as a `TransportError` -- the same path a normal connection failure takes. The `~OmniClient` `cancel()` logic is unchanged: `~OnDestructionCallback` fatals if destroyed while still scheduled, so the destructor must call `cancel()` regardless, and its return value already reports the destroyed state. The flag is additive for the send path, which cannot call `cancel()` (sends can repeat, and `cancel()` mutates/blocks). As with the destructor probe, this narrows but cannot fully close a concurrent destroy-during-send race -- inherent to a raw `EventBase` pointer. Reviewed By: prakashgayasen Differential Revision: D113842276 fbshipit-source-id: 946521c805c05fb90bddb28d567a2bf14c43daaagithub.com-facebook-fbthrift · 9dd76ab4 · 2026-07-28
- 0.4ETVEnable get_locally_set_fields via enable_isset_deprecated_unsafe Summary: This diff unifies the enablement of `get_locally_set_fields` and `isset_DEPRECATED` to satistfy the guarantee that if one is available, then the other is available. As of this diff, the followings rules apply: 1. Both are always disabled for exceptions and mutable structs 2. Both are enabled for all structs in a thrift library if the `enable_isset_deprecated_unsafe` compiler option is specified in the BUCK target. 3. Both are enabled for a specific struct if annotated with `python.EnableUnsafeIssetInspection`. This diff splits the test `thrift_library` into two parts: 1) library with the compiler option enabled, and 2) library without the compiler option enabled but with the struct-level annotations. It also ensures that all test cases assert equivalent behavior between `isset_DEPRECATED` and `get_locally_set_fields`. The next steps: 1. landing this diff and ensure it deploys in configerator compiler 2. switch the last usage of `isset_DEPRECATED` in configerator to `get_locally_set_fields` 3. remove `isset_DEPRECATED` from `types.pyx`. Reviewed By: prakashgayasen Differential Revision: D109470091 fbshipit-source-id: 6481fbc23631da42f8ae79b946fce9e5da4f8fbdgithub.com-facebook-fbthrift · 3ac10a4a · 2026-07-02
- 0.4ETVPreserve hostname in HTTP Host header for vhost routing Summary: The thrift-python HTTP client resolved the target hostname to an IP in the Python layer and then reused that single value both as the socket connect target and as the HTTP `Host` header. As a result, requests sent `Host: <resolved-ip>` instead of `Host: <hostname>`, breaking virtual-host routing on the server side. This threads the original hostname through as a separate `http_host` value alongside the connect `host` (which may be a resolved IP): - `createThriftChannelTCP` / `createThriftChannelSSL` (and their `sync_` variants) and `ConnectHandler` now take an `http_host` argument. The socket still connects to `host`, but the HTTP `Host` header (via `useAsHttpClient` / `setHTTPHost`) uses `http_host`. - The async and sync thrift-python channel factories preserve the original hostname and pass it as `http_host`; when the caller supplies an IP directly, `http_host` falls back to `host` (unchanged behavior). - The legacy `thrift.py3` client passes `http_host = host`, preserving its existing behavior. This is the follow-up to the test-only diff that documented the bug; the `test_http_client_preserves_hostname_for_virtual_host_routing` assertions are flipped back to assert the desired hostname-preserving behavior. Reviewed By: prakashgayasen Differential Revision: D113260616 fbshipit-source-id: 9d9798e4ec0398044af900a3e7cd286f1105af61github.com-facebook-fbthrift · 2047f8e4 · 2026-07-23
- 0.4ETVRename MutableStruct.fbthrift_copy_from to make shallow semantics explicit Summary: `MutableStruct.fbthrift_copy_from` is a SHALLOW copy: it assigns `other`'s internal `_fbthrift_data` slice directly, so `self` and `other` share references to nested structs and containers. The name `fbthrift_copy_from` reads like a full/deep copy, which is a footgun, and it is inconsistent with `MutableUnion.fbthrift_copy_from`, which is a deep copy. Rather than change the behavior (the shallow copy is intentional and relied upon for performance, e.g. Gaudi's insert-path snapshot), this renames the method to make the aliasing semantics impossible to miss: `MutableStruct.fbthrift_copy_from` -> `fbthrift_shallow_copy_I_KNOW_WHAT_IM_DOING` Callers must now explicitly acknowledge the shallow/aliasing contract. Updates the runtime (`.pyx`/`.pyi`) and all struct call sites (`dynamic_patch.py`, `base_dao.py`, `test_copy_from_correctness.py`). `MutableUnion.fbthrift_copy_from` (deep copy) is unchanged. It also adds coverage proving the shallow copy is safe with respect to the per-instance `_fbthrift_field_cache`, i.e. that attribute access never diverges from what serialization reads out of `_fbthrift_data`. In `struct_test.py`: - nested-struct aliasing, covering both when the nested instance is anchored in the shared data before the copy and when it is lazily created on first access after the copy (both first-access orderings); - container and `list<struct>` element sub-field mutation, in both the source->destination and destination->source directions; - adapted-field serialize/deserialize-vs-access consistency. `mutable_structs.py` gains a round-trip-vs-access check after an aliased nested mutation. Finally, it parameterizes the `StructDeepcopyTests` nested-struct copy tests (added in D78853414 as a regression for the D78775824 serialization drop) over the copy function, so `list<struct>`/`map<struct>` sub-field mutation is now verified for the shallow-copy API in addition to `copy.deepcopy`. The shallow copy does not exhibit the D78775824 bug: it aliases the source's internal data instead of producing an independent (and previously disconnected) copy, so there is only one representation and get/mutate/serialize stay consistent. Reviewed By: Mizuchi Differential Revision: D113804580 fbshipit-source-id: 52e0909dd6f2e5d235aeb32b60290e0efe44a23bgithub.com-facebook-fbthrift · 19c29783 · 2026-07-28
- 0.3ETVServer-side interactions: async termination hook Summary: Adds an optional async `onTermination` hook to thrift-python server-side interactions, invoked when an interaction ends so a per-session handler can release resources (flush buffers, close handles, etc.). Runtime-only -- no compiler/codegen change. - The `Interaction` base gains a default no-op `async def onTermination(self)`. Users override it on their interaction handler; interactions that do not override it are unaffected. - Runtime (`PythonTile`): overrides the framework hook `co_onTermination()` (fired on an explicit client terminate) to run `onTermination` on the Python executor and `co_await` it, so the hook completes before the interaction is torn down. If the connection drops *before* a terminate is received -- the case where the framework does not call `co_onTermination` -- the hook is instead fired best-effort from `~PythonTile`. An `std::atomic<bool> hookFired_` guarantees the hook runs at most once. The framework keeps the tile alive across `co_onTermination`, so the destructor always observes a settled flag (no race), and `Tile::onDestroy` is intentionally not used (it runs after the derived `PythonTile` members are destroyed). - Bridge (`python_async_processor.pyx`): `termination_coro` runs the hook, logging and swallowing exceptions (termination has no response channel). `handleInteractionTermination` (promise-bridged, awaited) and `scheduleInteractionTermination` (fire-and-forget) schedule it on the asyncio loop. Both tolerate a missing/closed loop: the awaited path completes its promise so `co_onTermination` cannot hang; the fire-and-forget path drops silently during shutdown. Reviewed By: prakashgayasen Differential Revision: D108062286 fbshipit-source-id: 245b55142140076817cecf9b175c582abf159b9bgithub.com-facebook-fbthrift · 4edf0acf · 2026-06-11
- 0.3ETVServer-side interactions: server streaming Summary: Extends thrift-python server-side interactions to support server-streaming methods declared inside an interaction (e.g. `i32, stream<i32> ticks(1: i32 count)`), dispatched against the correct per-session handler -- with or without an initial response, and able to raise declared stream exceptions. - Compiler (`services.whisker`): interaction handler interfaces now generate server-stream methods (the function-table gate relaxes from `not sink_or_stream?` to `not sink?`; sink/bidi inside interactions remain deferred). The interaction body rebinds `service` to the interaction so the shared service-side partials (`get_handler_result`, `m_functions.*`) resolve interaction struct names, and emits `_fbthrift__stream_wrapper_*` plus the `(serialize_iobuf(...), return_stream)` dispatch return. - Runtime (`PythonAsyncProcessor`): consolidates the interaction-routing step into `prepareInteractionDispatch`, shared by the request/response, stream, and oneway dispatch paths. It returns `folly::Try<PyObject*>` -- either the bound per-session handler (or `Py_None`) or the factory error -- so a stream-returning factory whose factory raises fails the request with the real message (matching the request/response path) instead of stranding the callback. - Tests: a `Counter` interaction with state-dependent stream methods (`ticks`, `drain`, `ticksThenFail`) plus a baseline service-level stream, covering streams with/without an initial response, a declared stream exception raised mid-stream, isolation across two concurrent interactions, and the explicit-factory exception path. Reviewed By: prakashgayasen Differential Revision: D107693699 fbshipit-source-id: c602e81709a352459f934784da219eb908df316fgithub.com-facebook-fbthrift · 8bb147ed · 2026-06-10
- 0.3ETVMove isset data to tuple tail Summary: Store immutable thrift-python isset bytes as a trailing tuple element instead of at index 0, so field values are always zero-based whether or not isset inspection is enabled. This removes the field-start offset from the Python and C++ runtime paths, updates the public inspection helpers to read the trailing bytes, and updates the Python CAPI codegen to emit zero-based tuple positions. It also collapses the two immutable tuple container policies into a single `TupleContainer`, making the trailing isset array an orthogonal `WithIsset` template parameter on the container-creation helpers rather than a separate type. Reviewed By: iahs Differential Revision: D110348797 fbshipit-source-id: 4293c5998c54361599a29da992c5e421748fc7a5github.com-facebook-fbthrift · 67377e9e · 2026-07-06
- 0.3ETVDowngrade convInt process-abort to DCHECK Summary: `convInt` in `types.cpp` used `LOG(FATAL)` to reject integer values that do not fit the target width, aborting the entire process. This mirrors the "CHECK to validate data" pattern that recently caused a SEV. The overflow case is not reachable through any public thrift-python API: every set/construct/insert path (immutable and mutable structs/unions, immutable and mutable containers, adapters, converter, and Any) validates integer bounds via `IntegerTypeInfo.to_internal_data` (raising `OverflowError`) before a value can be stored in `_fbthrift_data` and reach serialization. The only unvalidated writer, `_fbthrift_from_internal_data`, is fed exclusively already-validated (deserialized or copied) data. Since the guard protects a runtime invariant rather than user input, replace `LOG(FATAL)` with a `DCHECK` plus a defined truncating cast. In debug builds a broken invariant still fires loudly; in opt builds it degrades to a wrong integer value instead of crashing the whole server. Reviewed By: prakashgayasen Differential Revision: D113072780 fbshipit-source-id: 418fe794a596b3063f0e7de8da4e5fceb1258647github.com-facebook-fbthrift · a7d9241c · 2026-07-24
- 0.3ETVImport annotation-value type modules in services reflection Summary: Generated `*_thrift_services_reflection.py` files reference a type via its qualified `_fbthrift__..._thrift_types.<Name>` alias for every struct/union/enum literal nested inside a structured annotation value. The reflection generator, however, only emitted a function-scoped `import` for the annotation's own type module (e.g. `acl` for `acl.Action`), never for the modules of types nested inside the value. When an annotation value references a type from a program that is only transitively included -- e.g. `acl.Action{value = action.Value{...}}` where `action.thrift` is reachable only through the annotation value -- the generated reflection referenced `_fbthrift__...__action__thrift_types` without importing it, producing a runtime `NameError` when the service reflection was materialized (e.g. `MockFactory.create_default_mock` / `inspect`). Fix: add an `annotation_value_types` property on the thrift-python const-value prototype that walks the annotation value tree and collects the distinct types (one per defining program) referenced by struct/union/enum literals, excluding the root program (imported at file top) and the annotation's own type program (imported separately by the template). `thrift_services_reflection.py.whisker` now emits a function-scoped `import ... as ... # noqa: F811` for each, alongside the existing annotation-type import. Regression coverage: extended the `basic-structured-annotations` fixture with a `nested_annotation.thrift` file and an `included.annotation_with_nested_include{value = included.nested_annotation_value}` annotation on `MyService`, where `nested_annotation` is reachable only through the annotation value. Verified the generated reflection now imports `nested_annotation.thrift_types` and that reverting the generator change drops the import (reproducing the `NameError`). Reviewed By: createdbysk Differential Revision: D111518175 fbshipit-source-id: f1800f3115d568982b9311d03a46f478183fc927github.com-facebook-fbthrift · e085e3da · 2026-07-11
- 0.3ETVadd services reflection spec API Summary: Add `services_reflection.py` module with `FunctionSpec` and `ServiceSpec` classes for reflecting on thrift service definitions. Both classes use `_ImmutableMeta` for consistency with existing spec types (`FieldSpec`, `StructSpec`, etc.). `FunctionSpec` reuses `FieldSpec` for arguments and exceptions rather than introducing a separate `ParameterSpec`, since function parameters have the same shape as struct fields (id, name, type, qualifier, default, annotations). **`FunctionSpec`**: name, arguments (as `tuple[FieldSpec, ...]`), return type info, `is_oneway`, exceptions, structured annotations. **`ServiceSpec`**: name, functions (as `MappingProxyType[str, FunctionSpec]`), parent service (for inheritance), structured annotations. A follow-up diff will modify the thrift compiler to populate these specs in generated code and wire up `inspect()` for service clients and handlers. Reviewed By: prakashgayasen Differential Revision: D101863909 fbshipit-source-id: f3891bd6bd3eb86576269758ed21ddb0609ca846github.com-facebook-fbthrift · 4da6fa5f · 2026-05-22