[PR] Coverity: contain release() exceptions in ~ConnectingEntry [trafficserver]

["brbzull0 (via GitHub)" <gi...@apache.org>]

brbzull0 opened a new pull request, #13896:
URL: https://github.com/apache/trafficserver/pull/13896

   Since a19054e0a6 (#13545), `~ConnectingEntry()` calls `ConnectionTracker::Group::release()`, and `release()` is not `noexcept`. It locks the outbound table's `std::mutex`, which can throw `std::system_error`, and its `unordered_map::erase()` calls `Group::equal()`, whose debug line goes through libswoc formatting. The `std::runtime_error` in the report most likely comes from that second path, which statically reaches the two `throw std::runtime_error` sites in `lib/swoc/include/swoc/bwf_base.h`. Those throws need a capture spec or a nil name binding, and `BufferWriter::print()` produces neither, but the mutex lock is a real throw site. A destructor is implicitly `noexcept`, so an exception from either would end in `std::terminate()`.
   
   Coverity CID: 1688430
   
   **What changes:**
   
   - The `release()` call in the destructor is wrapped in `try`/`catch`. Both handlers call `ink_abort("exception escaped %s", __func__)`, the form #13590 uses in the plugin API, and the `std::exception` handler appends `what()`. The process still aborts if `release()` throws; the abort message now names the destructor and carries the exception text.
   - New `src/proxy/http/unit_tests/test_ConnectingEntry.cc`, built into `test_http`, covers the reservation the destructor holds:
     - "ConnectingEntry destructor returns the outbound reservation": deleting an entry that still holds its reservation, as a failed or cancelled connect does, brings the group's count back to zero and removes it from the outbound table.
     - "ConnectingEntry destructor releases exactly one reservation": with two reservations on one group, deleting the entry leaves one.
     - "ConnectingEntry destructor without connection tracking": an entry handed an empty group, as `HttpSM` does when tracking is off, leaves other reservations alone.
   
     These cases exercise what the destructor already did; they do not depend on the new guard.
   
   **What this PR does not do:**
   
   - It does not test the `catch` handlers. Making `release()` throw needs a broken mutex, and I found no portable way to get one in a unit test.
   - `~UnixNetVConnection()` reaches `Group::release()` the same way, through `release_inbound_connection_tracking()`. That path predates a19054e0a6 and is left as is.
   


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Coverity: annotate ~ConnectingEntry and test its reservation release [trafficserver]

["brbzull0 (via GitHub)" <gi...@apache.org>]

brbzull0 merged PR #13896:
URL: https://github.com/apache/trafficserver/pull/13896


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Coverity: annotate ~ConnectingEntry and test its reservation release [trafficserver]

["brbzull0 (via GitHub)" <gi...@apache.org>]

brbzull0 commented on PR #13896:
URL: https://github.com/apache/trafficserver/pull/13896#issuecomment-6437599421

   @masaori335 agreed, thanks. d13f77ea50 drops the try/catch and the `ink_error.h` include and annotates the `release()` call with your snippet as written. The unit tests stay; the no-tracking case no longer asserts on a reservation the entry never touches, so it is now a plain no-crash check.
   
   I've updated the description to match.
   


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org