moonchen commented on code in PR #13491:
URL: https://github.com/apache/trafficserver/pull/13491#discussion_r3716393270
##########
src/iocore/eventsystem/ConfigProcessor.cc:
##########
@@ -34,8 +37,69 @@ namespace
DbgCtl dbg_ctl_config{"config"};
+void
+destroy_config(unsigned int id, ConfigInfo *info)
+{
+ ink_hrtime start = ink_get_hrtime();
+
+ delete info;
+
+ if (dbg_ctl_config.on()) {
+ char thread_name[MAX_THREAD_NAME_LENGTH];
+
+ ink_get_thread_name(thread_name, sizeof(thread_name));
+ DbgPrint(dbg_ctl_config, "Destroyed config %d in %" PRId64 " ns on thread
%s", id, ink_get_hrtime() - start, thread_name);
+ }
+}
+
+/// Runs the destructor of a detached ConfigInfo on ET_TASK.
+class ConfigInfoDestroyer : public Continuation
+{
+public:
+ ConfigInfoDestroyer(unsigned int id, ConfigInfo *info) :
Continuation(new_ProxyMutex()), m_id(id), m_info(info)
+ {
+ SET_HANDLER(&ConfigInfoDestroyer::handle_event);
+ }
+
+ int
+ handle_event(int /* event ATS_UNUSED */, void * /* edata ATS_UNUSED */)
+ {
+ destroy_config(m_id, m_info);
+ delete this;
+ return EVENT_DONE;
+ }
+
+private:
+ unsigned int m_id;
+ ConfigInfo *m_info;
+};
+
+/// Hand a detached ConfigInfo to ET_TASK for destruction. Returns false when
the caller has to
+/// destroy it itself.
+bool
+destroy_config_on_task_thread(unsigned int id, ConfigInfo *info)
+{
+ EThread *ethread = this_ethread();
+
+ // Until the task threads are registered and spawned, ET_TASK is ET_CALL and
there is nowhere else
+ // to send this.
+ if (ethread == nullptr || ethread->is_event_type(ET_TASK) ||
eventProcessor.thread_group[ET_TASK]._count == 0) {
+ return false;
+ }
Review Comment:
This is a crash check rather than a style choice. Between
`TasksProcessor::register_event_type()` and `TasksProcessor::start()`, ET_TASK
is a distinct event type whose thread group is still empty, so
`assign_thread()` returns `thread_group[ET_TASK]._thread[0]`, which is null,
and `EventProcessor::schedule()` dereferences it when it enqueues. There is no
error return for that case.
`_count` is also the right field to read rather than a convenient proxy:
`spawn_event_threads()` populates `_thread[]` before assigning `_count`, so
`_count > 0` implies every pointer in the group is valid. The public accessor
`active_group_threads()` exposes only `begin()` and `end()`, so testing it is
less clear at the call site.
--
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]
