pjsua2: Add explicit AudioMediaPort callback fencing

[arch7tect]

Fixes #5308

Description

AudioMediaPort detaches its frame callbacks in its base destructor, after derived members have already been torn down. A derived implementation currently has no explicit fence to stop dispatch and drain in-flight callbacks while its callback-visible state is still valid.

This lifetime/race fix adds a small protected, non-virtual detachCallbacks() API using the existing group lock. The base destructor reuses it without changing its unregister/detach/destroy order. A second commit makes detached get_frame() return PJMEDIA_FRAME_TYPE_NONE and zero size instead of leaving the caller's frame metadata unchanged.

Two commits:

1. Extract and document the callback fence, with a regression test in the existing pjsua2-test target (GNU make and CMake).
2. Clear detached frame metadata, with stale-frame assertions.

Contract and motivation

• Permanently prevents the created port's get/put callbacks from dispatching into this object; repeated calls are safe. Before port creation, it is a no-op.
• Waits for in-flight callbacks when the caller does not already own the same recursive group lock. Calling from the callback itself is not a self-wait mechanism.
• Must be called while the object and callback-visible state are valid, before derived-member teardown. Explicit detach before destruction begins is the strict usage, especially with multiple inheritance levels; this does not promise generic concurrent destructor safety.
• Callers must not hold application, cross-port, or library locks needed by an in-flight callback.
• Does not unregister the port or destroy media resources.

No gateway-specific code, new object fields, virtual methods, or binding configuration changes are included. A single friend declaration gives the test access to the actual raw port without adding a public resource accessor.

Please consider inclusion in 2.18 if appropriate; acceptance of the API and release timing are up to the maintainers.

How Has This Been Tested?

Based on upstream master a67b8e81b0024b993f47e463c01c67c25cda116f; no equivalent fence was found there.

On macOS arm64 with Apple Clang 21 and SWIG 4.4.1:

• GNU configure/make build and full pjsua2-test: passed. The first commit was also compiled and tested independently.
• CMake shared-library build and ctest -R '^pjsua2-test$': passed.
• Python 3.13 SWIG extension build and import/Endpoint/AudioMediaPort smoke: passed.
• Java JNI and Java sources build, plus the existing Java smoke test: passed.
• C# SWIG generation and native wrapper compilation: passed; managed C# execution was not tested.
• Clang record-layout and vtable dumps match the base on this platform. Global-symbol comparison adds only pj::AudioMediaPort::detachCallbacks() to the media object; the shared library exports it. This is not a cross-platform ABI certification. The existing SWIG settings do not expose protected non-virtual methods, so this API is currently for C++ subclasses.

The regression test holds a real get/put callback at a promise barrier, starts a detacher on another registered thread, checks lock ownership and non-completion while the callback is held, then releases it and checks completion ordering. It checks repeated detach, retained registration, and suppression of subsequent callbacks in both directions. Detached reads are seeded with AUDIO/nonzero size and sentinel payload: they must return NONE/0. Payload bytes need not be erased because the result exposes no valid audio.

The blocking check uses a bounded completion wait after a start handshake; it does not instrument the mutex's internal waiter state. A scheduler that delays the detacher past the observation window can reduce that check's sensitivity. An additional callback-finished postcondition checks ordering. Local negative controls removing the fence lock or restoring stale-frame behavior both fail, while the corrected test passes.

Read-only Claude review covered reentrancy, recursive locking, lifetime/reference counts, destructor ordering, deadlocks, and API/binding impact. Its concrete test-ordering and cross-lock documentation suggestions were incorporated. No blocking findings remained.

Types of changes

• Bug fix with a small additive C++ API

Checklist

• Updated API documentation
• Added regression coverage
• Relevant local builds and PJSUA2 tests passed
• All platforms and the complete upstream test suite verified

[CLAassistant]

All committers have signed the CLA.

[nanangizz]

Thanks, both commits look correct to me. The fence is the destructor's existing code moved into its own method with the order unchanged, and since createPort() either creates the group lock or throws, the port->grp_lock check never silently skips on a created port. The test is well built: holding a real callback at a barrier and checking the completion order catches the regression without timing flakiness on the correct code.

Comments inline, mostly about who can call the fence and how it's documented. One more, outside the diff: the AudioMediaPort class doc (and ideally the pjsua2 guide) should say that implementations are expected to call the fence, otherwise few users will find it.

[nanangizz]

Was protected deliberate? It leaves out the class's main audience. A C++ app doesn't need AudioMediaPort: it can subclass AudioMedia, build its own pjmedia_port and register it with registerMediaPort2(), with full control over the callbacks and their lifetime. AudioMediaPort is mainly there so Java, Python and C# apps can implement a port through virtual callbacks, and as the description notes, SWIG doesn't expose protected non-virtual methods, so those apps can't reach the fence.

If it were public, they could call it before delete(). That path likely has the same race: the SWIG director's destructor runs before ~AudioMediaPort().

[nanangizz]

With a protected method, code outside the class hierarchy can't "explicitly detach before destruction begins". For a subclass, the simple rule that is always correct is: call it first thing in the most-derived class's destructor, which runs before any member or base is torn down. If the method becomes public (see above), the doc would need to cover both callers: application code calling it before deleting the object, or the most-derived destructor calling it first.

[nanangizz]

Minor: "permanently" doesn't hold when it's called before createPort(). It's a no-op then, and createPort() sets the back-pointer, so callbacks are dispatched afterwards. Something like "once the port has been created" would make that clear.

[nanangizz]

This fixes more than stale metadata. The conference bridge doesn't initialise f.type before calling get_frame(): read_port() in both conference.c and conf_thread.c sets only f.buf and f.size, on both the direct and the resampling paths, then reads f.type back. So on master, a detached get_frame() makes the bridge read an uninitialized frame type, and if it happens to equal AUDIO the bridge mixes whatever the buffer holds. That window exists today without the new API: ~AudioMediaPort() unregisters first, but the removal is asynchronous, so the bridge can still pull from the port after the back-pointer is cleared. Might be worth mentioning in the commit message.