hi amc,
Please consider there is 2 or more transformation plugins since the plugin does
not knows how many transform here.
If the 1st plugin perform TSVConnClose(output vc), the 2nd plugin will destroy
itself.
And the m_output_vc in the 1st plugin is not set to NULL.
Then, if HttpSM or HttpTunnel calls TVC->do_io_close(), the
INKVConnInternal::do_io_close() will close the m_output_vc of the 1st plugin
again, this will hit a double free.
The INKVConnInternal::do_io_close() is designed to perform a chain close on
m_output_vc.
According to the code, the TVC is a chain, the head is the 1st plugin, the tail
is Terminus, from the 2nd plugin to the last plugin are body. TVC is a
interface to manage the chain.
oknet xu
> 在 2017年3月30日，05:16，Alan Carroll <solidwallofc...@yahoo-inc.com> 写道：
>
>
> Sorry I have been so slow but getting back in to this is a bit of work. After
> a pass through the logic, I wonder if the case you are wondering about can
> occur because a plugin calls TSVConnClose on its outbound VConn. If that
> plugin is the one before the terminus (which is likely, because that's the
> case for a single transform), then the terminus can get closed before the
> TransformVC. I'll keep looking at this to dig a bit deeper.
> On Monday, March 6, 2017, 4:26:04 AM CST, Alan Carroll
> <solidwallofc...@yahoo-inc.com.INVALID> wrote:
> I'll try to take a look at this sometime this week.
>
>
> On Monday, March 6, 2017 2:46 AM, Chao Xu <xuc...@gmail.com> wrote:
>
>
> 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
>
>
