YIHONG-JIN commented on code in PR #11890:
URL: https://github.com/apache/trafficserver/pull/11890#discussion_r1928285656
##########
src/iocore/eventsystem/P_IOBuffer.h:
##########
@@ -656,6 +656,15 @@ new_MIOBuffer_internal(const char *location, int64_t
size_index)
TS_INLINE void
free_MIOBuffer(MIOBuffer *mio)
{
+#if TS_USE_LINUX_SPLICE
+ // check if mio is PipeIOBuffer using dynamic_cast
+ PipeIOBuffer *pipe_mio = dynamic_cast<PipeIOBuffer *>(mio);
Review Comment:
The PipeIOBuffer is now managed by pipeIOAllocator instead of ioAllocator. I
introduced pipeIOAllocator because it is a new class with additional
attributes. The ClassAllocator appears to be designed for concrete classes
rather than abstract classes or interfaces.
1. I didn’t move the cleanup logic to the virtual destructor because the
existing logic for MIOBuffer doesn’t use it. Instead, it manually handles
cleanup in free_MIOBuffer with the following steps:
```
mio->_writer = nullptr;
mio->dealloc_all_readers();
```
Interestingly, the destructor already performs this same cleanup, but for
some reason, the existing code avoids relying on it. If anyone knows why, I’d
appreciate the insight.
2. Even if we moved the cleanup logic to the virtual destructor, we would
still need a runtime type check since true polymorphism is not yet in place.
For example, when dealing with a PipeIOBuffer, we would need to use
```
THREAD_FREE(mio, pipeIOAllocator, this_thread());
```
instead of
```
THREAD_FREE(mio, ioAllocator, this_thread());
```
to make sure the correct allocator is used.
--
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-unsubscr...@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
