> 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.
>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>
