At some point pthreads may conceivably be made obsolete by a better thread library on Linux, MacOS or FreeBSD, and C++ standard <thread> would be ported to run on the better thread library. But true, unlikely to happen in ATS's lifetime.
On Wed, Mar 6, 2019 at 4:02 PM Leif Hedstrom <zw...@apache.org> wrote: > > > > On Mar 5, 2019, at 16:10, Walt Karas <wka...@verizonmedia.com.invalid> > wrote: > > > > What I was trying to say was, if we keep the thread handles abstract, we > > can switch from pthreads to stdc++ threads without impacting the plugins. > > Hmm, what’s the point / win in that? Likely c++ threads are an abstraction > on top of pthreads on many platforms no? So we’d do C abstractions (TS API) > on top of C++ abstractions (C++ threads) on top of C abstractions > (pthreads)? > > If we are going to change TS threads I feel fairly strongly that we should > eliminate the C abstractions in ATS. It was there to bridge platforms such > as Windows, and predates C++ threads. Then if it’s pthreads or C++ threads > is an open discussion. > > — Leif > > > >> On Tue, Mar 5, 2019 at 5:02 PM Leif Hedstrom <zw...@apache.org> wrote: > >> > >> > >> > >>> On Mar 5, 2019, at 2:24 PM, Walt Karas <wka...@verizonmedia.com > .INVALID> > >> wrote: > >>> > >>> Would we ever want to switch over to using libstdc++ threads? > >> > >> > >> I’m likely ok with that, but we have to figure out the story with > >> retaining C APIs. I.e. this is a big decision to make, making ts/ts.h > >> requiring C++ to compile. > >> > >> I’m not *opposed* to such a change; however, I feel it needs to be a > well > >> thought through and generally accepted community decision. This ties > into > >> other things, such as how to change TSDebug() to use the bwformat APIs. > >> > >> — Leif > >> > >>> > >>>> 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. > >>>>>>> > >>>>>> > >>>>>> > >>>> > >>>> > >> > >> > >