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 put the cleanup logic to the virtual destructor because the
existing logic for MIOBuffer doesn’t use MIOBuffer's destructor. Instead, it
manually handles cleanup in free_MIOBuffer with the following steps:
```
mio->_writer = nullptr;
mio->dealloc_all_readers();
```
Interestingly, the destructor is designed to perform 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]