Good points. And, is here even a reason to keep our own thread APIs? We only support posix threads anyways.
— Leif > On Feb 28, 2019, at 21:46, James Peach <jpe...@apache.org> wrote: > > > >> On Feb 28, 2019, at 9:17 AM, Fei Deng <duke8...@apache.org> wrote: >> >> void TSThreadSetCancelState(TSThread thread, int state); >> void TSThreadSetCancelType(TSThread thread, int type); > > I guess you are supposed to pass PTHREAD_CANCEL_* for type type parameter? I > don't like how this leaks the pthread implementation. An API should be > self-contained. you also don't capture the possible EINVAL return value here. > >> void TSThreadCancel(TSThread thread); > > I'm generally -1 on pthread cancellation since I feel like it is very hard to > use without leaking resources. Maybe your use case can be solved by an > in-band notification to the thread that it should exit (e.g. atomic variable, > massage send, file descriptor signal) that is then synchronized on the thread > join. > > Maybe diligent application of pthread_cleanup_push(3) can make cancellation > more robust, but that's not part of this proposal. > > FWIW I'd be sympathetic to a general API that just returns the underlying > pthread_t so you can color outside the lines if needed: > > void *TSThreadGetPlatformThread((TSThread) > >> void *TSThreadJoin(TSThread thread); > > This seems quite reasonable. > >> >> Some plugins have been causing a lot of crashes during ATS shutdown, the >> root cause is due plugin threads are not aware of ATS is shutting down and >> still trying to do stuff such as initiating ssl handshake. The workaround >> right now is to set a flag using the newly implemented >> `SHUTDOWN_LIFECYCLE_HOOK`, but there will still be race conditions since >> some threads have a very long turnaround time. >> >> These new APIs expose corresponding pthread calls so plugins can have a >> better control of its own threads. >
