amc, leif and bcall,

I have a question about the code in TransformTerminus::handle_event() while
I'm reading the ats code.

file: proxy/Transform.cc
```
 141 int
 142 TransformTerminus::handle_event(int event, void * /* edata ATS_UNUSED
*/)
 143 {
...
 158   if (m_closed != 0 && m_tvc->m_closed != 0) {
...
 164   } else if (m_write_vio.op == VIO::WRITE) {
...
 237   } else {
 238     MUTEX_TRY_LOCK(trylock2, m_read_vio.mutex, this_ethread());
 239     if (!trylock2.is_locked()) {
 240       RETRY();
 241     }
 242
 243     if (m_closed != 0) {
 244       // The terminus was closed, but the enclosing transform
 245       // vconnection wasn't. If the terminus was aborted then we
 246       // call the read_vio cont back with VC_EVENT_ERROR. If it
 247       // was closed normally then we call it back with
 248       // VC_EVENT_EOS. If a read operation hasn't been initiated
 249       // yet and we haven't called the user back then we call
 250       // the user back instead of the read_vio cont (which won't
 251       // exist).
 252       if (m_tvc->m_closed == 0) {
 253         int ev = (m_closed == TS_VC_CLOSE_ABORT) ? VC_EVENT_ERROR :
VC_EVENT_EOS;
 254
 255         if (!m_called_user) {
 256           m_called_user = 1;
 257           m_tvc->m_cont->handleEvent(ev, nullptr);
 258         } else {
 259           ink_assert(m_read_vio._cont != nullptr);
 260           m_read_vio._cont->handleEvent(ev, &m_read_vio);
 261         }
 262       }
 263
 264       return 0;
 265     }
 266   }
 267
 268   return 0;
 269 }
```

There is a comment from L244 to L251 that describe the terminus is already
closed but TransformVC do not close.

And in “TS-2578: Close the client connection when you close
TransformTerminus (ea382fd2c3179b1a00f5bd5cd364cc522768b7dd)” description,
plugin could do TSVConnAbort or TSVConnClose on output_vc.

According to my analysis for TransformVC, these 2 descriptions are not
correct.

I draw a graphic to understand the design of TransformVC:

    https://drive.google.com/open?id=0B6FSuL4tHtSHSVh6WGpSSXpoRTg

I found that:

1) TransformVC is a manager of VC chain and these VCs are chained
immediately after TransformVC created by TransformVC::TransformVC().
2) The TransformTerminus is the last VC in the chain and the rest is type
of INKVConnInternal and created by TSTransformCreate().
3) These INKVConnInternal are from hooked plugin list:
 api_hooks.get(TS_HTTP_REQUEST_TRANSFORM_HOOK) or
TS_HTTP_RESPONSE_TRANSFORM_HOOK.
4) The VC Chain is one way: the data write into the first INKVConnInternal
and read from the last TransformTerminus.
    a) The consumer of HttpTunnel call do_io_write on the first
INKVConnInternal by TVC::do_io_write and the HttpTunnel is the upstream of
the first INKVConnInternal.
    b) Downstream only inform upstream to send more data or complete data
receiving or error occurred,  the Transform Plugin must callback
m_write_vio._cont with WRITE_READY, WRITE_COMPLETE and ERROR event.
    c) Upstream is only able to perform do_io_write, do_io_shutdown,
reenable to downstream.
    c) the upstream must reenable() downstream while received WRITE_READY
event.
    d) the upstream must shutdown(WRITE) downstream while received
WRITE_COMPLETE event.
    e) the upstream must forward ERROR event to its upstream.
    f) the Transform Plugin must copy data from upstream to downstream
according to vio.nbytes and vio.ndone.
    g) as the last VC in the chain, the TransformTerminus must do the same
to upstream as Transform Plugins does and also callback m_read_vio._cont
with READ_READY, READ_COMPLETE, EOS and ERROR event to inform the producer
of HttpTunnel read data, just finished or eos/error occurred.
    h) the producer of HttpTunnel call do_io_read on TransformTerminus by
TVC::do_io_read and the HttpTunnel is the downstream of the
TransformTerminus.
    i) once the TransformTerminus got data from upstream, it will callback
TRANSFORM_READ_READY and the length of data to the user of TransformVC
(HttpSM).
    j) the HttpSM will create HttpTunnel chain to perform do_io_read on
TransformVC.
5) the do_io_close on TransformVC is forwarded one by one via
output_vc->do_io_close() in INKVConnInternal::do_io_close() and the every
VC in the chain is closed "at the same time" but they are
destroyed separately by Transform Plugin.
6) None of the Transform Plugin in the chain could perform do_io_close on
output_vc. it is could break the VC chain.
7) As the user/owner of TransformVC, HttpSM could perform do_io_close to
close the TransformVC chain, but should perform do_io_shutdown first.

According to the above,

1) the Transform Plugin must not do TSVConnAbort or TSVConnClose on
output_vc. (differ to TS-2578 description)
  I could not find any example code do TSVConnAbort or TSVConnClose on
TransformOutputVC.

  It will cause "heap use after free" if Transform Plugin do TSVConnClose
on TransformOutputVC then TransformVC::do_io_close() performed.  Because
INKVConnInternal::do_io_close() does not set output_vc to NULL after called
output_vc->do_io_close().

2) the TransformTerminus must not closed before TransformVC. (differ to the
comment from L244 to L251)
  I could not find any example code that close the Terminus directly.

Am I right ?


Thanks!

Oknet Xu

Reply via email to