Skip to content

Commit ef2017f

Browse files
simpsont-ocisimpsont-oci
committed
resolve review comments
1 parent e42c099 commit ef2017f

9 files changed

Lines changed: 88 additions & 80 deletions

‎ACE/ace/Asynch_IO_Impl.cpp‎

Lines changed: 25 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,12 @@ ACE_Asynch_Read_Stream_Impl::~ACE_Asynch_Read_Stream_Impl (void)
2525
}
2626

2727
int
28-
ACE_Asynch_Read_Stream_Impl::readv (ACE_Message_Block &message_block,
29-
size_t bytes_to_read,
30-
const void *act,
31-
int priority,
32-
int signal_number)
28+
ACE_Asynch_Read_Stream_Impl::readv (ACE_Message_Block &/*message_block*/,
29+
size_t /*bytes_to_read*/,
30+
const void */*act*/,
31+
int /*priority*/,
32+
int /*signal_number*/)
3333
{
34-
ACE_UNUSED_ARG (message_block);
35-
ACE_UNUSED_ARG (bytes_to_read);
36-
ACE_UNUSED_ARG (act);
37-
ACE_UNUSED_ARG (priority);
38-
ACE_UNUSED_ARG (signal_number);
3934
errno = ENOTSUP;
4035
return -1;
4136
}
@@ -49,17 +44,12 @@ ACE_Asynch_Write_Stream_Impl::~ACE_Asynch_Write_Stream_Impl (void)
4944
}
5045

5146
int
52-
ACE_Asynch_Write_Stream_Impl::writev (ACE_Message_Block &message_block,
53-
size_t bytes_to_write,
54-
const void *act,
55-
int priority,
56-
int signal_number)
57-
{
58-
ACE_UNUSED_ARG (message_block);
59-
ACE_UNUSED_ARG (bytes_to_write);
60-
ACE_UNUSED_ARG (act);
61-
ACE_UNUSED_ARG (priority);
62-
ACE_UNUSED_ARG (signal_number);
47+
ACE_Asynch_Write_Stream_Impl::writev (ACE_Message_Block &/*message_block*/,
48+
size_t /*bytes_to_write*/,
49+
const void */*act*/,
50+
int /*priority*/,
51+
int /*signal_number*/)
52+
{
6353
errno = ENOTSUP;
6454
return -1;
6555
}
@@ -73,21 +63,14 @@ ACE_Asynch_Read_File_Impl::~ACE_Asynch_Read_File_Impl (void)
7363
}
7464

7565
int
76-
ACE_Asynch_Read_File_Impl::readv (ACE_Message_Block &message_block,
77-
size_t bytes_to_read,
78-
u_long offset,
79-
u_long offset_high,
80-
const void *act,
81-
int priority,
82-
int signal_number)
66+
ACE_Asynch_Read_File_Impl::readv (ACE_Message_Block &/*message_block*/,
67+
size_t /*bytes_to_read*/,
68+
u_long /*offset*/,
69+
u_long /*offset_high*/,
70+
const void */*act*/,
71+
int /*priority*/,
72+
int /*signal_number*/)
8373
{
84-
ACE_UNUSED_ARG (message_block);
85-
ACE_UNUSED_ARG (bytes_to_read);
86-
ACE_UNUSED_ARG (offset);
87-
ACE_UNUSED_ARG (offset_high);
88-
ACE_UNUSED_ARG (act);
89-
ACE_UNUSED_ARG (priority);
90-
ACE_UNUSED_ARG (signal_number);
9174
errno = ENOTSUP;
9275
return -1;
9376
}
@@ -113,21 +96,14 @@ ACE_Asynch_Write_File_Impl::~ACE_Asynch_Write_File_Impl (void)
11396
}
11497

11598
int
116-
ACE_Asynch_Write_File_Impl::writev (ACE_Message_Block &message_block,
117-
size_t bytes_to_write,
118-
u_long offset,
119-
u_long offset_high,
120-
const void *act,
121-
int priority,
122-
int signal_number)
99+
ACE_Asynch_Write_File_Impl::writev (ACE_Message_Block &/*message_block*/,
100+
size_t /*bytes_to_write*/,
101+
u_long /*offset*/,
102+
u_long /*offset_high*/,
103+
const void */*act*/,
104+
int /*priority*/,
105+
int /*signal_number*/)
123106
{
124-
ACE_UNUSED_ARG (message_block);
125-
ACE_UNUSED_ARG (bytes_to_write);
126-
ACE_UNUSED_ARG (offset);
127-
ACE_UNUSED_ARG (offset_high);
128-
ACE_UNUSED_ARG (act);
129-
ACE_UNUSED_ARG (priority);
130-
ACE_UNUSED_ARG (signal_number);
131107
errno = ENOTSUP;
132108
return -1;
133109
}

‎ACE/ace/Asynch_Pseudo_Task.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ ACE_BEGIN_VERSIONED_NAMESPACE_DECL
88
ACE_Asynch_Pseudo_Task::ACE_Asynch_Pseudo_Task ()
99
: select_reactor_ (), // should be initialized before reactor_
1010
reactor_ (&select_reactor_, 0), // don't delete implementation
11-
started_ (0)
11+
started_ (false)
1212
{
1313
}
1414

@@ -20,7 +20,7 @@ ACE_Asynch_Pseudo_Task::~ACE_Asynch_Pseudo_Task ()
2020
int
2121
ACE_Asynch_Pseudo_Task::start (void)
2222
{
23-
if (this->started_.value () != 0)
23+
if (this->started_.value ())
2424
return 0;
2525

2626
if (this->reactor_.initialized () == 0)
@@ -32,22 +32,22 @@ ACE_Asynch_Pseudo_Task::start (void)
3232
if (this->activate () == -1)
3333
return -1;
3434

35-
this->started_ = 1;
35+
this->started_ = true;
3636
return 0;
3737
}
3838

3939
int
4040
ACE_Asynch_Pseudo_Task::stop (void)
4141
{
42-
if (this->started_.value () == 0) // already stopped
42+
if (!this->started_.value ()) // already stopped
4343
return 0;
4444

4545
if (this->reactor_.end_reactor_event_loop () == -1)
4646
return -1;
4747

4848
this->wait ();
4949
this->reactor_.close ();
50-
this->started_ = 0;
50+
this->started_ = false;
5151
return 0;
5252
}
5353

‎ACE/ace/Asynch_Pseudo_Task.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ class ACE_Export ACE_Asynch_Pseudo_Task : public ACE_Task<ACE_NULL_SYNCH>
6565
ACE_Atomic_Op<ACE_SYNCH_MUTEX, bool> started_;
6666
};
6767

68+
6869
ACE_END_VERSIONED_NAMESPACE_DECL
6970

7071
#include /**/ "ace/post.h"

‎ACE/ace/POSIX_CB_Proactor.cpp‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@ ACE_BEGIN_VERSIONED_NAMESPACE_DECL
1111

1212
ACE_POSIX_CB_Proactor::Notification_State::Notification_State (ACE_SYNCH_SEMAPHORE &sema)
1313
: sema_ (&sema),
14-
pending_callbacks_ (0)
14+
pending_callbacks_ (0),
15+
ref_count_ (0)
1516
{
1617
}
1718

1819
void
1920
ACE_POSIX_CB_Proactor::Notification_State::add_pending (void)
2021
{
22+
this->add_ref ();
2123
++this->pending_callbacks_;
2224
}
2325

@@ -26,6 +28,7 @@ ACE_POSIX_CB_Proactor::Notification_State::complete_one (void)
2628
{
2729
this->sema_->release ();
2830
--this->pending_callbacks_;
31+
this->remove_ref ();
2932
}
3033

3134
long
@@ -34,13 +37,27 @@ ACE_POSIX_CB_Proactor::Notification_State::pending (void) const
3437
return this->pending_callbacks_.value ();
3538
}
3639

40+
void
41+
ACE_POSIX_CB_Proactor::Notification_State::add_ref (void)
42+
{
43+
++this->ref_count_;
44+
}
45+
46+
void
47+
ACE_POSIX_CB_Proactor::Notification_State::remove_ref (void)
48+
{
49+
if (--this->ref_count_ == 0)
50+
delete this;
51+
}
52+
3753
ACE_POSIX_CB_Proactor::ACE_POSIX_CB_Proactor (size_t max_aio_operations)
3854
: ACE_POSIX_AIOCB_Proactor (max_aio_operations,
3955
ACE_POSIX_Proactor::PROACTOR_CB),
4056
sema_ ((unsigned int) 0),
4157
notification_state_ (0)
4258
{
4359
ACE_NEW (this->notification_state_, Notification_State (this->sema_));
60+
this->notification_state_->add_ref ();
4461

4562
// we should start pseudo-asynchronous accept task
4663
// one per all future acceptors
@@ -79,24 +96,23 @@ ACE_POSIX_CB_Proactor_aio_completion (sigval cb_data)
7996
int
8097
ACE_POSIX_CB_Proactor::close (void)
8198
{
82-
const int result = ACE_POSIX_AIOCB_Proactor::close ();
99+
int const result = ACE_POSIX_AIOCB_Proactor::close ();
83100

84101
Notification_State *state = this->notification_state_;
85102
if (state != 0)
86103
{
87-
const ACE_Time_Value settle_interval (0, 10000);
88-
const size_t max_settle_attempts = 50;
104+
this->notification_state_ = 0;
105+
106+
ACE_Time_Value const settle_interval (0, 10000);
107+
size_t const max_settle_attempts = 50;
89108

90109
for (size_t attempt = 0;
91110
state->pending () > 0 && attempt < max_settle_attempts;
92111
++attempt)
93112
ACE_OS::sleep (settle_interval);
94113

95-
if (state->pending () == 0)
96-
{
97-
delete state;
98-
this->notification_state_ = 0;
99-
}
114+
state->detach ();
115+
state->remove_ref ();
100116
}
101117

102118
return result;
@@ -117,10 +133,8 @@ ACE_POSIX_CB_Proactor::handle_events (void)
117133
}
118134

119135
int
120-
ACE_POSIX_CB_Proactor::notify_completion (int sig_num)
136+
ACE_POSIX_CB_Proactor::notify_completion (int /* sig_num */)
121137
{
122-
ACE_UNUSED_ARG (sig_num);
123-
124138
return this->sema_.release();
125139
}
126140

@@ -262,28 +276,31 @@ ACE_POSIX_CB_Proactor::start_aio (ACE_POSIX_Asynch_Result *result,
262276
return -1;
263277
}
264278

265-
const ssize_t slot = this->allocate_aio_slot (result);
279+
ssize_t const slot = this->allocate_aio_slot (result);
266280
if (slot < 0)
267281
return -1;
268282

269-
const size_t index = static_cast<size_t> (slot);
283+
size_t const index = static_cast<size_t> (slot);
270284
this->result_list_[index] = result;
271285
++this->aiocb_list_cur_size_;
272286

287+
if (this->notification_state_ != 0)
288+
this->notification_state_->add_pending ();
289+
273290
ret_val = this->start_aio_i (result);
274291
switch (ret_val)
275292
{
276293
case 0:
277294
this->aiocb_list_[index] = result;
278-
if (this->notification_state_ != 0)
279-
this->notification_state_->add_pending ();
280295
return 0;
281296

282297
case 1:
283298
++this->num_deferred_aiocb_;
284299
return 0;
285300

286301
default:
302+
if (this->notification_state_ != 0)
303+
this->notification_state_->complete_one ();
287304
break;
288305
}
289306

‎ACE/ace/POSIX_CB_Proactor.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,16 @@ class ACE_Export ACE_POSIX_CB_Proactor : public ACE_POSIX_AIOCB_Proactor
4747
void complete_one (void);
4848
long pending (void) const;
4949

50+
void add_ref (void);
51+
void remove_ref (void);
52+
53+
void detach (void);
54+
5055
private:
56+
ACE_Thread_Mutex mutex_;
5157
ACE_SYNCH_SEMAPHORE *sema_;
5258
ACE_Atomic_Op<ACE_Thread_Mutex, long> pending_callbacks_;
59+
ACE_Atomic_Op<ACE_Thread_Mutex, long> ref_count_;
5360
};
5461

5562
public:

‎ACE/ace/POSIX_Proactor.cpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,9 +1073,7 @@ ACE_POSIX_AIOCB_Proactor::handle_events (ACE_Time_Value &wait_time)
10731073
int
10741074
ACE_POSIX_AIOCB_Proactor::handle_events (void)
10751075
{
1076-
// Avoid an unbounded wait here so the event loop can observe
1077-
// end_event_loop() even if the notify-pipe wakeup is lost.
1078-
return this->handle_events_i (1000);
1076+
return this->handle_events_i (ACE_INFINITE);
10791077
}
10801078

10811079
int
@@ -2112,8 +2110,9 @@ ACE_POSIX_Wakeup_Completion::complete (size_t /* bytes_transferred */,
21122110
const void * /* completion_key */,
21132111
u_long /* error */)
21142112
{
2115-
// This completion exists only to wake blocked event-loop threads.
2116-
// Once it reaches dispatch, the wakeup has already served its purpose.
2113+
ACE_Handler *handler = this->handler_proxy_.get ()->handler ();
2114+
if (handler != 0)
2115+
handler->handle_wakeup ();
21172116
}
21182117

21192118
ACE_END_VERSIONED_NAMESPACE_DECL

‎ACE/ace/Proactor.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,13 @@ class ACE_Proactor_Timer_Handler : public ACE_Task<ACE_NULL_SYNCH>
8787
ACE_Proactor &proactor_;
8888

8989
/// Flag used to indicate when we are shutting down.
90-
ACE_Atomic_Op<ACE_Thread_Mutex, int> shutting_down_;
90+
ACE_Atomic_Op<ACE_Thread_Mutex, bool> shutting_down_;
9191
};
9292

9393
ACE_Proactor_Timer_Handler::ACE_Proactor_Timer_Handler (ACE_Proactor &proactor)
9494
: ACE_Task <ACE_NULL_SYNCH> (&proactor.thr_mgr_),
9595
proactor_ (proactor),
96-
shutting_down_ (0)
96+
shutting_down_ (false)
9797
{
9898
}
9999

@@ -105,7 +105,7 @@ ACE_Proactor_Timer_Handler::~ACE_Proactor_Timer_Handler (void)
105105
int
106106
ACE_Proactor_Timer_Handler::destroy (void)
107107
{
108-
this->shutting_down_ = 1;
108+
this->shutting_down_ = true;
109109

110110
// Signal timer event.
111111
this->timer_event_.signal ();

‎ACE/ace/Uring_Proactor.cpp‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ int
8484
ACE_Uring_Proactor::handle_events (ACE_Time_Value &wait_time)
8585
{
8686
ACE_Countdown_Time countdown (&wait_time);
87-
const int result = this->process_cqes (DEFAULT_CQE_BATCH_SIZE, &wait_time);
87+
int const result = this->process_cqes (DEFAULT_CQE_BATCH_SIZE, &wait_time);
8888
return result > 0 ? 1 : result;
8989
}
9090

@@ -161,6 +161,9 @@ ACE_Uring_Proactor::process_cqes (int max_to_process, const ACE_Time_Value *wait
161161
{
162162
ACE_GUARD_RETURN (ACE_Thread_Mutex, guard, this->cq_mutex_, -1);
163163

164+
if (!this->is_initialized_)
165+
return -1;
166+
164167
if (should_wait)
165168
{
166169
if (wait_time != 0)
@@ -263,7 +266,7 @@ ACE_Uring_Proactor::submit_sqe_if_necessary (void)
263266
if (!this->is_initialized_)
264267
return -1;
265268

266-
const unsigned int ready = queued_sqes (this->ring_);
269+
unsigned int const ready = queued_sqes (this->ring_);
267270
if (ready == 0)
268271
return 0;
269272

@@ -576,6 +579,10 @@ int
576579
ACE_Uring_Proactor::post_wakeup_completions (int count)
577580
{
578581
ACE_GUARD_RETURN (ACE_Thread_Mutex, guard, this->sq_mutex_, -1);
582+
583+
if (!this->is_initialized_)
584+
return -1;
585+
579586
for (int i = 0; i < count; ++i)
580587
{
581588
struct io_uring_sqe *sqe = ::io_uring_get_sqe (&this->ring_);

0 commit comments

Comments
 (0)