Would we ever want to switch over to using libstdc++ threads? On Tue, Mar 5, 2019 at 3:12 PM Leif Hedstrom <zw...@apache.org> wrote:
> I still think it might be time to investigate if eliminating this > abstraction is better. Like I said, we only support posix threads anyways > so this abstraction is just extra overhead I assume ? > > — Leif > > > On Mar 4, 2019, at 09:57, Fei Deng <f...@verizonmedia.com.invalid> > wrote: > > > > I don't like setting cancel state and type myself either, but at the > time I > > couldn't come up with other ways of making sure the thread are in the > right > > state/type. I think the idea just returning a pthread_t to the plugin is > a > > good idea, it can also solve the problem with the set state/type, I > didn't > > know we are allowed to return that information since we wrapped it so > deep. > > > >> On Thu, Feb 28, 2019 at 11:02 PM Leif Hedstrom <zw...@apache.org> > wrote: > >> > >> 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. > >>> > >> > >> > >
