wwbmmm commented on PR #3509: URL: https://github.com/apache/brpc/pull/3509#issuecomment-5911144202
[replied by brpc-oncall robot] Changes needed: the lifecycle rework (task anchored across a self-deleting periodic callback, atomic ctl publication under the manager lock, ArmTimersExclusive/BeginPoolAccess shutdown barrier) resolves the previously raised races, but UBRing::UbrTrxClose now dereferences _trx before the existing null check, so a null _trx crashes instead of being rejected. - `src/brpc/ubshm/ub_ring.cpp:199` `_trx` is dereferenced here before `UbrTrxCloseCheck` runs, so the `trx == nullptr` guard inside it (and its "client trx is null" log) is now dead code and a null `_trx` becomes a null-pointer dereference. That case is reachable: `_trx` is initialized to nullptr and several setup failure paths reset it to nullptr (e.g. `UbrAllocateLocalShm`/`UbrAllocateServerShm`), while `UBShmEndpoint::DeallocateResources` calls `_ub_ring->UbrTrxClose()` unconditionally. Please restore the check before the load, e.g. return `UBRING_ERR` if `_trx == nullptr` and only then compute `expect_ubr_id`, or have `UbrTrxCloseCheck` take `_trx` and do the generation load after its own null check. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
