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

Reply via email to