Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
6b7bc8f
Added new timer class for idle transport purging
jwillemsen Mar 26, 2026
c7bab82
Use TAO namespace
jwillemsen Mar 26, 2026
e61960a
Commandline flag
jwillemsen Mar 26, 2026
4313626
Schedule timer on making idle
jwillemsen Mar 26, 2026
9c8c4b3
Add cancel timer
jwillemsen Mar 26, 2026
cacbcdb
Added cancel of idle timer on transport usage
jwillemsen Mar 26, 2026
8798127
Add missing post include
jwillemsen Mar 27, 2026
c718855
Starter unit test
jwillemsen Mar 27, 2026
fbc35c8
Test extension
jwillemsen Mar 27, 2026
adbd880
Storing work in progress
jwillemsen Mar 27, 2026
266e5b0
Add new test
jwillemsen Mar 28, 2026
30d5cb4
Test improvements and cleanup
jwillemsen Mar 30, 2026
d57b5ec
Move add/remove ref to idle timer
jwillemsen Mar 30, 2026
7f66766
Document new flag
jwillemsen Mar 30, 2026
5e8ce3d
Docu changes
jwillemsen Mar 30, 2026
5fafd77
Cleanup
jwillemsen Mar 30, 2026
00ff507
fixed fguzz
jwillemsen Mar 30, 2026
b93d274
Rework test for more features
jwillemsen Mar 31, 2026
2b8198c
Test infrastructure extensions
jwillemsen Mar 31, 2026
0e7d800
Removed disabled test, default
jwillemsen Mar 31, 2026
31046bd
Test infrastructure
jwillemsen Mar 31, 2026
c7c77cb
Add special client for testing with multiple servers, mixing basic an…
jwillemsen Mar 31, 2026
1dfa8fe
Test extensions
jwillemsen Mar 31, 2026
e0c745a
Layout/nullptr changes
jwillemsen Mar 31, 2026
ef7fd51
Work in progress
jwillemsen Apr 1, 2026
101a7ef
Always schedule the timer
jwillemsen Apr 1, 2026
71166a8
Test fixes
jwillemsen Apr 1, 2026
db2dd46
Rework for thread safety
jwillemsen Apr 1, 2026
630ec1e
Removed empty line
jwillemsen Apr 3, 2026
b9fbb69
Update TAO/tests/Transport_Idle_Timeout/run_test.pl
jwillemsen Apr 3, 2026
def90e1
Update TAO/tests/Transport_Idle_Timeout/svc.conf
jwillemsen Apr 3, 2026
b94d846
fixe
jwillemsen Apr 3, 2026
94df85a
Merge branch 'jwi-idletransport' of https://github.com/jwillemsen/ATC…
jwillemsen Apr 3, 2026
cc41da5
Return value changes
jwillemsen Apr 3, 2026
7c944d4
Revert change
jwillemsen Apr 3, 2026
9ca5ffb
Removed todo
jwillemsen Apr 3, 2026
812c76e
Removed not used is_idle
jwillemsen Apr 3, 2026
d6d34dc
Fixed typo
jwillemsen Apr 3, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions TAO/bin/tao_orb_tests.lst
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,7 @@ TAO/tests/TransportCurrent/Framework/run_test.pl -static: !DISABLE_TRANSPORT_CUR
TAO/tests/TransportCurrent/IIOP/run_test.pl -dynamic: !DISABLE_TRANSPORT_CURRENT !STATIC !CORBA_E_COMPACT !CORBA_E_MICRO !DISABLE_INTERCEPTORS !MINIMUM
TAO/tests/TransportCurrent/IIOP/run_test.pl -static: !DISABLE_TRANSPORT_CURRENT STATIC !CORBA_E_COMPACT !CORBA_E_MICRO !DISABLE_INTERCEPTORS !MINIMUM
TAO/tests/Transport_Cache_Manager/run_test.pl
TAO/tests/Transport_Idle_Timeout/run_test.pl
TAO/tests/UNKNOWN_Exception/run_test.pl:
TAO/tests/Native_Exceptions/run_test.pl:
TAO/tests/Servant_To_Reference_Test/run_test.pl: !MINIMUM !CORBA_E_COMPACT !CORBA_E_MICRO !ST
Expand Down
6 changes: 6 additions & 0 deletions TAO/docs/Options.html
Original file line number Diff line number Diff line change
Expand Up @@ -1388,6 +1388,12 @@ <h4><a name="TDRF">1.1. Resource_Factory</a></h4>
to your config.h file.
</td>
</tr>
<tr>
<td><code>-ORBTransportIdleTimeout</code> <em>number</em></td>
<td>Number of seconds after which idle connections are closed and purged from
the transport cache. By default idle connections are kept until they are
explicitly closed or purged because the transport cache is almost full.</td>
</tr>
</tbody>
</table>
</p>
Expand Down
7 changes: 3 additions & 4 deletions TAO/tao/Cache_Entries_T.inl
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ namespace TAO
{
template <typename TRANSPORT_TYPE> ACE_INLINE
Cache_IntId_T<TRANSPORT_TYPE>::Cache_IntId_T ()
: transport_ (0),
: transport_ (nullptr),
recycle_state_ (ENTRY_UNKNOWN),
is_connected_ (false)
{
}

template <typename TRANSPORT_TYPE> ACE_INLINE
Cache_IntId_T<TRANSPORT_TYPE>::Cache_IntId_T (const Cache_IntId_T &rhs)
: transport_ (0),
: transport_ (nullptr),
recycle_state_ (ENTRY_UNKNOWN),
is_connected_ (false)
{
Expand Down Expand Up @@ -75,7 +75,7 @@ namespace TAO
{
// Yield ownership of the TAO_Transport object.
transport_type *val = this->transport_;
this->transport_ = 0;
this->transport_ = nullptr;
return val;
}

Expand Down Expand Up @@ -112,7 +112,6 @@ namespace TAO
is_delete_ (false),
index_ (0)
{

}

template <typename TRANSPORT_DESCRIPTOR_TYPE> ACE_INLINE
Expand Down
2 changes: 2 additions & 0 deletions TAO/tao/Resource_Factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,8 @@ class TAO_Export TAO_Resource_Factory : public ACE_Service_Object
/// replies during shutdown.
virtual bool drop_replies_during_shutdown () const = 0;

virtual int transport_idle_timeout () const = 0;

protected:
/**
* Loads the default protocols. This method is used so that the
Expand Down
143 changes: 125 additions & 18 deletions TAO/tao/Transport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ TAO_Transport::TAO_Transport (CORBA::ULong tag,
, tail_ (nullptr)
, incoming_message_queue_ (orb_core)
, current_deadline_ (ACE_Time_Value::zero)
, flush_timer_id_ (-1)
, transport_timer_ (this)
, transport_idle_timer_ (this)
, handler_lock_ (orb_core->resource_factory ()->create_cached_connection_lock ())
, id_ ((size_t) this)
, purging_order_ (0)
Expand Down Expand Up @@ -183,10 +183,12 @@ TAO_Transport::~TAO_Transport ()
{
if (TAO_debug_level > 9)
{
TAOLIB_DEBUG ((LM_DEBUG, ACE_TEXT ("TAO (%P|%t) - Transport[%d]::~Transport\n"),
TAOLIB_DEBUG ((LM_DEBUG, ACE_TEXT ("TAO (%P|%t) - Transport[%d]::~Transport, start\n"),
this->id_));
}

this->cancel_idle_timer ();

delete this->messaging_object_;

delete this->ws_;
Expand Down Expand Up @@ -217,6 +219,11 @@ TAO_Transport::~TAO_Transport ()
#if TAO_HAS_TRANSPORT_CURRENT == 1
delete this->stats_;
#endif /* TAO_HAS_TRANSPORT_CURRENT == 1 */
if (TAO_debug_level > 9)
{
TAOLIB_DEBUG ((LM_DEBUG, ACE_TEXT ("TAO (%P|%t) - Transport[%d]::~Transport, end\n"),
this->id_));
}
}

void
Expand Down Expand Up @@ -324,6 +331,16 @@ TAO_Transport::register_if_necessary ()
void
TAO_Transport::close_connection ()
{
if (TAO_debug_level > 4)
{
TAOLIB_DEBUG ((LM_DEBUG,
ACE_TEXT ("TAO (%P|%t) - Transport[%d]::close_connection\n"),
this->id ()));
}

// Cancel any pending timer
this->cancel_idle_timer ();

this->connection_handler_i ()->close_connection ();
}

Expand Down Expand Up @@ -375,8 +392,7 @@ TAO_Transport::register_handler ()
this->ws_->is_registered (true);

// Register the handler with the reactor
return r->register_handler (this->event_handler_i (),
ACE_Event_Handler::READ_MASK);
return r->register_handler (this->event_handler_i (), ACE_Event_Handler::READ_MASK);
}

int
Expand Down Expand Up @@ -547,12 +563,24 @@ TAO_Transport::make_idle ()
this->id ()));
}

return this->transport_cache_manager ().make_idle (this->cache_map_entry_);
int const result = this->transport_cache_manager ().make_idle (this->cache_map_entry_);
if (result == 0)
{
this->schedule_idle_timer ();
}
return result;
}

int
TAO_Transport::update_transport ()
{
if (TAO_debug_level > 3)
{
TAOLIB_DEBUG ((LM_DEBUG,
ACE_TEXT ("TAO (%P|%t) - Transport[%d]::update_transport\n"),
this->id ()));
}

return this->transport_cache_manager ().update_entry (this->cache_map_entry_);
}

Expand Down Expand Up @@ -852,7 +880,7 @@ TAO_Transport::schedule_output_i ()
ACE_Event_Handler * const eh = this->event_handler_i ();
ACE_Reactor * const reactor = eh->reactor ();

if (reactor == nullptr)
if (!reactor)
{
if (TAO_debug_level > 1)
{
Expand Down Expand Up @@ -940,23 +968,63 @@ TAO_Transport::handle_timeout (const ACE_Time_Value & /* current_time */,
// pending.
this->reset_flush_timer ();

TAO_Flushing_Strategy *flushing_strategy =
this->orb_core ()->flushing_strategy ();
TAO_Flushing_Strategy *flushing_strategy = this->orb_core ()->flushing_strategy ();
int const result = flushing_strategy->schedule_output (this);
if (result == TAO_Flushing_Strategy::MUST_FLUSH)
{
typedef ACE_Reverse_Lock<ACE_Lock> TAO_REVERSE_LOCK;
TAO_REVERSE_LOCK reverse (*this->handler_lock_);
ACE_GUARD_RETURN (TAO_REVERSE_LOCK, ace_mon, reverse, -1);
if (flushing_strategy->flush_transport (this, nullptr) == -1) {
return -1;
}
if (flushing_strategy->flush_transport (this, nullptr) == -1)
{
return -1;
}
}
}

return 0;
}

int
TAO_Transport::handle_idle_timeout (const ACE_Time_Value & /* current_time */, const void */*act*/)
{
if (TAO_debug_level > 6)
{
TAOLIB_DEBUG ((LM_DEBUG,
ACE_TEXT ("TAO (%P|%t) - Transport[%d]::handle_idle_timeout, ")
ACE_TEXT ("idle timer expired, closing transport\n"),
this->id ()));
}

// Timer has expired, so setting the idle timer id back to -1
this->idle_timer_id_ = -1;

if (this->transport_cache_manager ().purge_entry_when_purgable (this->cache_map_entry_) == -1)
{
if (TAO_debug_level > 6)
TAOLIB_DEBUG ((LM_DEBUG,
ACE_TEXT ("TAO (%P|%t) - Transport[%d]::handle_idle_timeout, ")
ACE_TEXT ("idle_timeout, transport is not purgable, don't close it, reschedule it\n"),
this->id ()));

this->schedule_idle_timer ();
}
else
{
if (TAO_debug_level > 6)
TAOLIB_DEBUG ((LM_DEBUG,
ACE_TEXT ("TAO (%P|%t) - Transport[%d]::handle_idle_timeout, ")
ACE_TEXT ("idle_timeout, transport purged due to idle timeout\n"),
this->id ()));

// Close the underlying socket.
// close_connection() is safe to call from the reactor thread.
(void) this->close_connection ();
}

return 0;
}
Comment on lines +988 to +1026
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Version the idle-timer state; idle_timer_id_ alone is racy.

handle_idle_timeout() currently ignores act, while schedule_idle_timer(), cancel_idle_timer(), and the reactor callback all read/write idle_timer_id_ from different threads. If an old timeout is already dequeued when reuse cancels/reschedules the transport, a stale callback or canceller can overwrite the newer ID and leave the replacement timer registered while the transport thinks no timer is pending. Because transport_idle_timer_ is embedded in TAO_Transport, that orphaned reactor entry can then outlive the transport and dispatch through a dangling handler. Please serialize the timer state and gate callbacks with a generation token so stale firings become no-ops.

Also applies to: 2942-2983

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@TAO/tao/Transport.cpp` around lines 989 - 1027, handle_idle_timeout is racy
because idle_timer_id_ alone can be overwritten by concurrent schedule/cancel
operations; add a generation token (e.g. idle_timer_gen_ as an atomic/integer)
and include that token in the reactor act when scheduling timers in
schedule_idle_timer and cancel_idle_timer so the reactor callback and cancel
compare generation before mutating state. On schedule: increment
idle_timer_gen_, store the new generation with idle_timer_id_ and pass the
generation as act; on cancel: only clear idle_timer_id_ if the passed/observed
generation matches and otherwise leave newer timers alone; in
handle_idle_timeout(const ACE_Time_Value&, const void *act) extract and compare
the generation in act to the current idle_timer_gen_ and return no-op if they
differ, otherwise proceed with the existing
purge_entry_when_purgable/close_connection logic; protect updates to
idle_timer_id_ and idle_timer_gen_ with the existing transport lock or use
atomics to serialize access.


TAO_Transport::Drain_Result
TAO_Transport::drain_queue (TAO::Transport::Drain_Constraints const & dc)
{
Expand Down Expand Up @@ -2729,8 +2797,7 @@ TAO_Transport::pre_close ()
// of the is_connected_ flag, so that during cache lookups the cache
// manager doesn't need to be burdened by the lock in is_connected().
this->is_connected_ = false;
this->transport_cache_manager ().mark_connected (this->cache_map_entry_,
false);
this->transport_cache_manager ().mark_connected (this->cache_map_entry_, false);
this->purge_entry ();
{
ACE_MT (ACE_GUARD (ACE_Lock, guard, *this->handler_lock_));
Expand Down Expand Up @@ -2804,13 +2871,10 @@ TAO_Transport::post_open (size_t id)
ACE_TEXT (", cache_map_entry_ is [%@]\n"), this->id_, this->cache_map_entry_));
}

this->transport_cache_manager ().mark_connected (this->cache_map_entry_,
true);
this->transport_cache_manager ().mark_connected (this->cache_map_entry_, true);

// update transport cache to make this entry available
this->transport_cache_manager ().set_entry_state (
this->cache_map_entry_,
TAO::ENTRY_IDLE_AND_PURGABLE);
this->transport_cache_manager ().set_entry_state (this->cache_map_entry_, TAO::ENTRY_IDLE_AND_PURGABLE);

return true;
}
Expand Down Expand Up @@ -2874,4 +2938,47 @@ TAO_Transport::connection_closed_on_read () const
return connection_closed_on_read_;
}

void
TAO_Transport::schedule_idle_timer ()
{
int const timeout_sec = this->orb_core_->resource_factory ()->transport_idle_timeout ();
if (timeout_sec > 0)
{
ACE_Reactor *reactor = this->orb_core_->reactor ();
const ACE_Time_Value tv (static_cast<time_t> (timeout_sec));
this->idle_timer_id_= reactor->schedule_timer (std::addressof(this->transport_idle_timer_), nullptr, tv);

if (TAO_debug_level > 6)
{
TAOLIB_DEBUG ((LM_DEBUG,
ACE_TEXT ("TAO (%P|%t) - Transport[%d]::schedule_idle_timer, ")
ACE_TEXT ("schedule idle timer with id [%d] ")
ACE_TEXT ("in the reactor.\n"),
this->id (), this->idle_timer_id_));
}
}
}

void
TAO_Transport::cancel_idle_timer ()
{
if (this->idle_timer_id_ != -1)
{
ACE_Reactor *reactor = this->orb_core ()->reactor ();
if (reactor)
{
if (TAO_debug_level > 6)
{
TAOLIB_DEBUG ((LM_DEBUG,
ACE_TEXT ("TAO (%P|%t) - Transport[%d]::cancel_idle_timer, ")
ACE_TEXT ("cancel idle timer with id [%d] ")
ACE_TEXT ("from the reactor.\n"),
this->id (), this->idle_timer_id_));
}
reactor->cancel_timer (this->idle_timer_id_);
this->idle_timer_id_ = -1;
}
}
}

TAO_END_VERSIONED_NAMESPACE_DECL
27 changes: 17 additions & 10 deletions TAO/tao/Transport.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// -*- C++ -*-

//=============================================================================
/**
* @file Transport.h
*
Expand All @@ -9,7 +6,6 @@
*
* @author Fred Kuhns <fredk@cs.wustl.edu>
*/
//=============================================================================

#ifndef TAO_TRANSPORT_H
#define TAO_TRANSPORT_H
Expand All @@ -23,6 +19,7 @@
#endif /* ACE_LACKS_PRAGMA_ONCE */

#include "tao/Transport_Timer.h"
#include "tao/Transport_Idle_Timer.h"
#include "tao/Incoming_Message_Queue.h"
#include "tao/Incoming_Message_Stack.h"
#include "tao/Message_Semantics.h"
Expand Down Expand Up @@ -853,6 +850,9 @@ class TAO_Export TAO_Transport
*/
int handle_timeout (const ACE_Time_Value &current_time, const void* act);

/// Timeout called when the idle timer expired for this transport
int handle_idle_timeout (const ACE_Time_Value &current_time, const void* act);

/// Accessor to recv_buffer_size_
size_t recv_buffer_size () const;

Expand Down Expand Up @@ -896,6 +896,9 @@ class TAO_Export TAO_Transport
/// Transport statistics
TAO::Transport::Stats* stats () const;

/// Helper method to cancel the timer when the transport is not idle anymore
void cancel_idle_timer ();

private:
/// Helper method that returns the Transport Cache Manager.
TAO::Transport_Cache_Manager &transport_cache_manager ();
Expand Down Expand Up @@ -1058,10 +1061,8 @@ class TAO_Export TAO_Transport
*/
bool using_blocking_io_for_asynch_messages() const;

/*
* Specialization hook to add concrete private methods from
* TAO's protocol implementation onto the base Transport class
*/
/// Helper method to schedule a timer when the transport is made idle
void schedule_idle_timer ();

protected:
/// IOP protocol tag.
Expand Down Expand Up @@ -1120,10 +1121,16 @@ class TAO_Export TAO_Transport
ACE_Time_Value current_deadline_;

/// The timer ID
long flush_timer_id_;
long flush_timer_id_ { -1 };

/// The idle timer ID
long idle_timer_id_ { -1 };

/// The adapter used to receive timeout callbacks from the Reactor
TAO_Transport_Timer transport_timer_;
TAO::Transport_Timer transport_timer_;

/// The adapter used to receive idle timeout callbacks from the Reactor
TAO::Transport_Idle_Timer transport_idle_timer_;

/// Lock that insures that activities that *might* use handler-related
/// resources (such as a connection handler) get serialized.
Expand Down
Loading
Loading