You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Emmanuel Lécharny <el...@gmail.com> on 2022/03/02 06:05:13 UTC
SSL handler and SSLEngine tasks
Hi Jonathan,
I wonder if using an executor to process SSLEngine tasks is useful at
all, considering that the execute_task method is synchronized:
protected void schedule_task(NextFilter next) {
if (ENABLE_ASYNC_TASKS) {
if (mExecutor == null) {
execute_task(next);
} else {
mExecutor.execute(new Runnable() {
@Override
public void run() {
SSLHandlerG0.this.execute_task(next);
}
});
and
synchronized protected void execute_task(NextFilter next) {
...
Also the execute_task will pull tasks from the SSLEngine - we may have
more than one, so it will loop on the tasks - and it's unlikely that we
will have more tasks to execute when exiting the execute_tasks method.
The only benefit I can see is that the main thread will be freed for
other tasks - like processing another IoSession - while tasks are being
processed by another thread. And may be this is what was intended, but I
wonder iff we should not simply spawn a dedicated thread for that purpose.
IMO, if we want to introduce an executor in the equation, it should be
done in the execute_task method, not in the schedule_task, it would
speed up the TLS processing, while also freeing the main thread fast
enough - even a bit slower than having the task loop being processed by
a dedicated thread.
wdyt?
--
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecharny@busit.com https://www.busit.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
Re: SSL handler and SSLEngine tasks
Posted by Jonathan Valliere <jo...@apache.org>.
*Cores
On Wed, Mar 2, 2022 at 8:31 PM Jonathan Valliere <jo...@apache.org> wrote:
> I was using an Executor to limit the number of threads that consume this
> action to prevent the server from an SSL attack spinning on all the codes.
>
> On Wed, Mar 2, 2022 at 6:43 PM Emmanuel Lécharny <el...@gmail.com>
> wrote:
>
>> FTR, there is no point in using an executor to try to spread the load of
>> tasks across many threads:
>>
>> private static class DelegatedTask implements Runnable {
>> private final SSLEngineImpl engine;
>>
>> DelegatedTask(SSLEngineImpl engineInstance) {
>> this.engine = engineInstance;
>> }
>>
>> @Override
>> public void run() {
>> synchronized (engine) { <<------ This...
>>
>> Whatever we do, when a task is executed, it will block any other
>> DelegatedTask.
>>
>>
>> On 02/03/2022 15:33, Emmanuel Lécharny wrote:
>> >
>> >
>> > On 02/03/2022 13:57, Jonathan Valliere wrote:
>> >>
>> >> CONFIDENTIALITY NOTICE: The contents of this email message and any
>> >> attachments are intended solely for the addressee(s) and may contain
>> >> confidential and/or privileged information and may be legally
>> >> protected from disclosure.
>> >>
>> >>
>> >> On Wed, Mar 2, 2022 at 7:33 AM Emmanuel Lécharny <elecharny@gmail.com
>> >> <ma...@gmail.com>> wrote:
>> >>
>> >>
>> >>
>> >> On 02/03/2022 12:36, Jonathan Valliere wrote:
>> >> > It’s already running in an Executor
>> >>
>> >> Can you clarify ? What is running in an executor?
>> >>
>> >>
>> >> The SSLEngine delegated task is run in a thread pool executor outside
>> >> of the filterchain.
>> >>
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L617
>> >> <
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L617>
>>
>> >>
>> >>
>> >>
>> >> which is why it’s a runnable
>> >>
>> >> Well, it's not related. You can create Runnable instances and
>> execute
>> >> them outside of an executor.
>> >>
>> >> . The
>> >> > Runnable defers the actual execution logic to the execute task
>> >> function
>> >> > which should be the one that is looping to perform all queued
>> >> tasks from
>> >> > the SSL Engine.
>> >>
>> >> Yes, but my point is that we could spawn threads inside the execute
>> >> task
>> >> (one thread per task) instead of creating an executor inside the
>> >> schedule_task.
>> >>
>> >>
>> >> It uses a configured Executor instance which is generally the same one
>> >> for every SSL session.
>> >>
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L84
>> >> <
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L84>
>>
>> >>
>> >>
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLFilter.java#L72
>> >> <
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLFilter.java#L72>
>>
>> >>
>> >
>> > Ok, so we are on the same page here.
>> >
>> > So the goal here is to spawn a thread that will process the
>> > delegatedTask and let the IoProcessor deal with another session.
>> >
>> > That's pretty much needed considering the potentially costly task done
>> > in the delegated task.
>> >
>> >
>> >
>> >>
>> >>
>> >> My fear is that bu creating a new thread for each schedule_task
>> >> call is
>> >> that we may have many pending threads waiting for the thread
>> >> executing
>> >> execute_task to be completed, because of the synchronized nature
>> >> of the
>> >> execute_task method. All in all, it seems to create a bottleneck
>> >> that is
>> >> going to be problematic, when the tasks are supposed to be ran
>> >> concurrently.
>> >>
>> >> Am I missing something ?
>> >>
>> >>
>> >> If more than one scheduled task could happen (which I doubt it
>> >> honestly) yes they would block on each other because the execute_task
>> >> function is synchronized. But we want to block here because the
>> >> delegated tasks MUST be executed in order,
>> >
>> > No, this is not required:
>> >
>> > "Multiple delegated tasks can be run in parallel."
>> > (
>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/javax/net/ssl/SSLEngine.html#getDelegatedTask())
>>
>> >
>> >
>> > Although it's not clear if they meant "multiple delegated tasks for
>> > different sessions" or "multiple delegated tasks for one session", I
>> > think the former is correct.
>> >
>> > The SSLEngine will manage the context, and any wrap/unwrap call will be
>> > blocked until all the tasks are executed.
>> >
>> > So my guess is that it makes sense to use another executor to process
>> > the tasks, such as in my proposal.
>> >
>> > Now, going back to the initial question - ie why do we need an executor
>> > when we execute a synchronized method - do you agree that my
>> > understanding is correct: this allow the IoProcessor thread to be freed
>> > for another session ?
>> >
>> >
>> > if a second scheduled task is created, it's a
>> >> guaranteed that any subsequent delegated tasks are actually executed
>> >> and not lost in the time between a previous execution of a delegated
>> >> task and the exit of the synchronized block in execute_task. We don't
>> >> want any weird race conditions here.
>> >
>> > I'm not sure we could face race condition when executing tasks. They
>> are
>> > supposed to be safe from a concurrency PoV.
>> >
>> >
>> >>
>> >> I wanted to ensure that the async captured exception elevates to the
>> >> Filter exception handler in a very standard and known way.
>> >
>> > Now this is interesting. OTOH as soon as we catch an exception for any
>> > task being executed, we will process it. It *may* happen we have more
>> > than one, if we have more than one task being executed concurrently.
>> But
>> > in this case, the session would just be shut down by the first
>> exception
>> > being handled, no matter which one it is.
>> >
>> > A simple way
>> >> to force the handling of the exception on the filter-chain is to cause
>> >> a throwing of a special WriteRequest which will flow down the filter
>> >> chain and cause the error to be thrown. You already see where I'm
>> >>
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L473
>> >> <
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L473>
>>
>> >> writing the EncryptedWriteRequest from the async execute_task
>> >> function. Just create an EncryptedErrorRequest or some other object to
>> >> write and force the trigger.
>> >
>> > I'm not sure that is needed. Actually, the exception will pop up the
>> > chain, and the Handler will get it. What is important, and what is
>> > currently missing, is the alert that is not writen back to the remote
>> > peer (but that is the other thread's discussion).
>> >
>> >
>> >
>> > SSLEngine is a kind of nightmare...
>> >
>>
>> --
>> *Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
>> T. +33 (0)4 89 97 36 50
>> P. +33 (0)6 08 33 32 61
>> emmanuel.lecharny@busit.com https://www.busit.com/
>>
>
Re: SSL handler and SSLEngine tasks
Posted by Emmanuel Lécharny <el...@gmail.com>.
And IMO this is a good idea. It decouples the task execution from the
IoProcessor.
Now, I don't think it will help against a SSL attack: the tasks will be
queued once the max threads limit is reached, which means it may take
forever to unqueue them once the attack is over, sucking all your CPU,
eventually ending with a OOM if the queue grew too large. Such attacks
should be mitigated at another level (You'll need some FW or dedicated
software to do that)
Otherwise I specifically referred to my proposal to use another executor
to spawn the tasks to be processed across threads: it won't work because
of the synchronized(engine). So my understanding of the "Multiple
delegated tasks can be run in parallel." sentence was incorrect: it
means you can run multiple delegated tasks as soon as they are for
different SSL sessions...
Bottom line, it's now clear for me and this thread can die :-)
On 03/03/2022 02:31, Jonathan Valliere wrote:
> I was using an Executor to limit the number of threads that consume this
> action to prevent the server from an SSL attack spinning on all the codes.
>
> On Wed, Mar 2, 2022 at 6:43 PM Emmanuel Lécharny <el...@gmail.com>
> wrote:
>
>> FTR, there is no point in using an executor to try to spread the load of
>> tasks across many threads:
>>
>> private static class DelegatedTask implements Runnable {
>> private final SSLEngineImpl engine;
>>
>> DelegatedTask(SSLEngineImpl engineInstance) {
>> this.engine = engineInstance;
>> }
>>
>> @Override
>> public void run() {
>> synchronized (engine) { <<------ This...
>>
>> Whatever we do, when a task is executed, it will block any other
>> DelegatedTask.
>>
>>
>> On 02/03/2022 15:33, Emmanuel Lécharny wrote:
>>>
>>>
>>> On 02/03/2022 13:57, Jonathan Valliere wrote:
>>>>
>>>> CONFIDENTIALITY NOTICE: The contents of this email message and any
>>>> attachments are intended solely for the addressee(s) and may contain
>>>> confidential and/or privileged information and may be legally
>>>> protected from disclosure.
>>>>
>>>>
>>>> On Wed, Mar 2, 2022 at 7:33 AM Emmanuel Lécharny <elecharny@gmail.com
>>>> <ma...@gmail.com>> wrote:
>>>>
>>>>
>>>>
>>>> On 02/03/2022 12:36, Jonathan Valliere wrote:
>>>> > It’s already running in an Executor
>>>>
>>>> Can you clarify ? What is running in an executor?
>>>>
>>>>
>>>> The SSLEngine delegated task is run in a thread pool executor outside
>>>> of the filterchain.
>>>>
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L617
>>>> <
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L617>
>>
>>>>
>>>>
>>>>
>>>> which is why it’s a runnable
>>>>
>>>> Well, it's not related. You can create Runnable instances and
>> execute
>>>> them outside of an executor.
>>>>
>>>> . The
>>>> > Runnable defers the actual execution logic to the execute task
>>>> function
>>>> > which should be the one that is looping to perform all queued
>>>> tasks from
>>>> > the SSL Engine.
>>>>
>>>> Yes, but my point is that we could spawn threads inside the execute
>>>> task
>>>> (one thread per task) instead of creating an executor inside the
>>>> schedule_task.
>>>>
>>>>
>>>> It uses a configured Executor instance which is generally the same one
>>>> for every SSL session.
>>>>
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L84
>>>> <
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L84>
>>
>>>>
>>>>
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLFilter.java#L72
>>>> <
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLFilter.java#L72>
>>
>>>>
>>>
>>> Ok, so we are on the same page here.
>>>
>>> So the goal here is to spawn a thread that will process the
>>> delegatedTask and let the IoProcessor deal with another session.
>>>
>>> That's pretty much needed considering the potentially costly task done
>>> in the delegated task.
>>>
>>>
>>>
>>>>
>>>>
>>>> My fear is that bu creating a new thread for each schedule_task
>>>> call is
>>>> that we may have many pending threads waiting for the thread
>>>> executing
>>>> execute_task to be completed, because of the synchronized nature
>>>> of the
>>>> execute_task method. All in all, it seems to create a bottleneck
>>>> that is
>>>> going to be problematic, when the tasks are supposed to be ran
>>>> concurrently.
>>>>
>>>> Am I missing something ?
>>>>
>>>>
>>>> If more than one scheduled task could happen (which I doubt it
>>>> honestly) yes they would block on each other because the execute_task
>>>> function is synchronized. But we want to block here because the
>>>> delegated tasks MUST be executed in order,
>>>
>>> No, this is not required:
>>>
>>> "Multiple delegated tasks can be run in parallel."
>>> (
>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/javax/net/ssl/SSLEngine.html#getDelegatedTask())
>>
>>>
>>>
>>> Although it's not clear if they meant "multiple delegated tasks for
>>> different sessions" or "multiple delegated tasks for one session", I
>>> think the former is correct.
>>>
>>> The SSLEngine will manage the context, and any wrap/unwrap call will be
>>> blocked until all the tasks are executed.
>>>
>>> So my guess is that it makes sense to use another executor to process
>>> the tasks, such as in my proposal.
>>>
>>> Now, going back to the initial question - ie why do we need an executor
>>> when we execute a synchronized method - do you agree that my
>>> understanding is correct: this allow the IoProcessor thread to be freed
>>> for another session ?
>>>
>>>
>>> if a second scheduled task is created, it's a
>>>> guaranteed that any subsequent delegated tasks are actually executed
>>>> and not lost in the time between a previous execution of a delegated
>>>> task and the exit of the synchronized block in execute_task. We don't
>>>> want any weird race conditions here.
>>>
>>> I'm not sure we could face race condition when executing tasks. They are
>>> supposed to be safe from a concurrency PoV.
>>>
>>>
>>>>
>>>> I wanted to ensure that the async captured exception elevates to the
>>>> Filter exception handler in a very standard and known way.
>>>
>>> Now this is interesting. OTOH as soon as we catch an exception for any
>>> task being executed, we will process it. It *may* happen we have more
>>> than one, if we have more than one task being executed concurrently. But
>>> in this case, the session would just be shut down by the first exception
>>> being handled, no matter which one it is.
>>>
>>> A simple way
>>>> to force the handling of the exception on the filter-chain is to cause
>>>> a throwing of a special WriteRequest which will flow down the filter
>>>> chain and cause the error to be thrown. You already see where I'm
>>>>
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L473
>>>> <
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L473>
>>
>>>> writing the EncryptedWriteRequest from the async execute_task
>>>> function. Just create an EncryptedErrorRequest or some other object to
>>>> write and force the trigger.
>>>
>>> I'm not sure that is needed. Actually, the exception will pop up the
>>> chain, and the Handler will get it. What is important, and what is
>>> currently missing, is the alert that is not writen back to the remote
>>> peer (but that is the other thread's discussion).
>>>
>>>
>>>
>>> SSLEngine is a kind of nightmare...
>>>
>>
>> --
>> *Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
>> T. +33 (0)4 89 97 36 50
>> P. +33 (0)6 08 33 32 61
>> emmanuel.lecharny@busit.com https://www.busit.com/
>>
>
--
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecharny@busit.com https://www.busit.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
Re: SSL handler and SSLEngine tasks
Posted by Jonathan Valliere <jo...@apache.org>.
I was using an Executor to limit the number of threads that consume this
action to prevent the server from an SSL attack spinning on all the codes.
On Wed, Mar 2, 2022 at 6:43 PM Emmanuel Lécharny <el...@gmail.com>
wrote:
> FTR, there is no point in using an executor to try to spread the load of
> tasks across many threads:
>
> private static class DelegatedTask implements Runnable {
> private final SSLEngineImpl engine;
>
> DelegatedTask(SSLEngineImpl engineInstance) {
> this.engine = engineInstance;
> }
>
> @Override
> public void run() {
> synchronized (engine) { <<------ This...
>
> Whatever we do, when a task is executed, it will block any other
> DelegatedTask.
>
>
> On 02/03/2022 15:33, Emmanuel Lécharny wrote:
> >
> >
> > On 02/03/2022 13:57, Jonathan Valliere wrote:
> >>
> >> CONFIDENTIALITY NOTICE: The contents of this email message and any
> >> attachments are intended solely for the addressee(s) and may contain
> >> confidential and/or privileged information and may be legally
> >> protected from disclosure.
> >>
> >>
> >> On Wed, Mar 2, 2022 at 7:33 AM Emmanuel Lécharny <elecharny@gmail.com
> >> <ma...@gmail.com>> wrote:
> >>
> >>
> >>
> >> On 02/03/2022 12:36, Jonathan Valliere wrote:
> >> > It’s already running in an Executor
> >>
> >> Can you clarify ? What is running in an executor?
> >>
> >>
> >> The SSLEngine delegated task is run in a thread pool executor outside
> >> of the filterchain.
> >>
> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L617
> >> <
> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L617>
>
> >>
> >>
> >>
> >> which is why it’s a runnable
> >>
> >> Well, it's not related. You can create Runnable instances and
> execute
> >> them outside of an executor.
> >>
> >> . The
> >> > Runnable defers the actual execution logic to the execute task
> >> function
> >> > which should be the one that is looping to perform all queued
> >> tasks from
> >> > the SSL Engine.
> >>
> >> Yes, but my point is that we could spawn threads inside the execute
> >> task
> >> (one thread per task) instead of creating an executor inside the
> >> schedule_task.
> >>
> >>
> >> It uses a configured Executor instance which is generally the same one
> >> for every SSL session.
> >>
> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L84
> >> <
> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L84>
>
> >>
> >>
> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLFilter.java#L72
> >> <
> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLFilter.java#L72>
>
> >>
> >
> > Ok, so we are on the same page here.
> >
> > So the goal here is to spawn a thread that will process the
> > delegatedTask and let the IoProcessor deal with another session.
> >
> > That's pretty much needed considering the potentially costly task done
> > in the delegated task.
> >
> >
> >
> >>
> >>
> >> My fear is that bu creating a new thread for each schedule_task
> >> call is
> >> that we may have many pending threads waiting for the thread
> >> executing
> >> execute_task to be completed, because of the synchronized nature
> >> of the
> >> execute_task method. All in all, it seems to create a bottleneck
> >> that is
> >> going to be problematic, when the tasks are supposed to be ran
> >> concurrently.
> >>
> >> Am I missing something ?
> >>
> >>
> >> If more than one scheduled task could happen (which I doubt it
> >> honestly) yes they would block on each other because the execute_task
> >> function is synchronized. But we want to block here because the
> >> delegated tasks MUST be executed in order,
> >
> > No, this is not required:
> >
> > "Multiple delegated tasks can be run in parallel."
> > (
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/javax/net/ssl/SSLEngine.html#getDelegatedTask())
>
> >
> >
> > Although it's not clear if they meant "multiple delegated tasks for
> > different sessions" or "multiple delegated tasks for one session", I
> > think the former is correct.
> >
> > The SSLEngine will manage the context, and any wrap/unwrap call will be
> > blocked until all the tasks are executed.
> >
> > So my guess is that it makes sense to use another executor to process
> > the tasks, such as in my proposal.
> >
> > Now, going back to the initial question - ie why do we need an executor
> > when we execute a synchronized method - do you agree that my
> > understanding is correct: this allow the IoProcessor thread to be freed
> > for another session ?
> >
> >
> > if a second scheduled task is created, it's a
> >> guaranteed that any subsequent delegated tasks are actually executed
> >> and not lost in the time between a previous execution of a delegated
> >> task and the exit of the synchronized block in execute_task. We don't
> >> want any weird race conditions here.
> >
> > I'm not sure we could face race condition when executing tasks. They are
> > supposed to be safe from a concurrency PoV.
> >
> >
> >>
> >> I wanted to ensure that the async captured exception elevates to the
> >> Filter exception handler in a very standard and known way.
> >
> > Now this is interesting. OTOH as soon as we catch an exception for any
> > task being executed, we will process it. It *may* happen we have more
> > than one, if we have more than one task being executed concurrently. But
> > in this case, the session would just be shut down by the first exception
> > being handled, no matter which one it is.
> >
> > A simple way
> >> to force the handling of the exception on the filter-chain is to cause
> >> a throwing of a special WriteRequest which will flow down the filter
> >> chain and cause the error to be thrown. You already see where I'm
> >>
> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L473
> >> <
> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L473>
>
> >> writing the EncryptedWriteRequest from the async execute_task
> >> function. Just create an EncryptedErrorRequest or some other object to
> >> write and force the trigger.
> >
> > I'm not sure that is needed. Actually, the exception will pop up the
> > chain, and the Handler will get it. What is important, and what is
> > currently missing, is the alert that is not writen back to the remote
> > peer (but that is the other thread's discussion).
> >
> >
> >
> > SSLEngine is a kind of nightmare...
> >
>
> --
> *Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
> T. +33 (0)4 89 97 36 50
> P. +33 (0)6 08 33 32 61
> emmanuel.lecharny@busit.com https://www.busit.com/
>
Re: SSL handler and SSLEngine tasks
Posted by Emmanuel Lécharny <el...@gmail.com>.
FTR, there is no point in using an executor to try to spread the load of
tasks across many threads:
private static class DelegatedTask implements Runnable {
private final SSLEngineImpl engine;
DelegatedTask(SSLEngineImpl engineInstance) {
this.engine = engineInstance;
}
@Override
public void run() {
synchronized (engine) { <<------ This...
Whatever we do, when a task is executed, it will block any other
DelegatedTask.
On 02/03/2022 15:33, Emmanuel Lécharny wrote:
>
>
> On 02/03/2022 13:57, Jonathan Valliere wrote:
>>
>> CONFIDENTIALITY NOTICE: The contents of this email message and any
>> attachments are intended solely for the addressee(s) and may contain
>> confidential and/or privileged information and may be legally
>> protected from disclosure.
>>
>>
>> On Wed, Mar 2, 2022 at 7:33 AM Emmanuel Lécharny <elecharny@gmail.com
>> <ma...@gmail.com>> wrote:
>>
>>
>>
>> On 02/03/2022 12:36, Jonathan Valliere wrote:
>> > It’s already running in an Executor
>>
>> Can you clarify ? What is running in an executor?
>>
>>
>> The SSLEngine delegated task is run in a thread pool executor outside
>> of the filterchain.
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L617
>> <https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L617>
>>
>>
>>
>> which is why it’s a runnable
>>
>> Well, it's not related. You can create Runnable instances and execute
>> them outside of an executor.
>>
>> . The
>> > Runnable defers the actual execution logic to the execute task
>> function
>> > which should be the one that is looping to perform all queued
>> tasks from
>> > the SSL Engine.
>>
>> Yes, but my point is that we could spawn threads inside the execute
>> task
>> (one thread per task) instead of creating an executor inside the
>> schedule_task.
>>
>>
>> It uses a configured Executor instance which is generally the same one
>> for every SSL session.
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L84
>> <https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L84>
>>
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLFilter.java#L72
>> <https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLFilter.java#L72>
>>
>
> Ok, so we are on the same page here.
>
> So the goal here is to spawn a thread that will process the
> delegatedTask and let the IoProcessor deal with another session.
>
> That's pretty much needed considering the potentially costly task done
> in the delegated task.
>
>
>
>>
>>
>> My fear is that bu creating a new thread for each schedule_task
>> call is
>> that we may have many pending threads waiting for the thread
>> executing
>> execute_task to be completed, because of the synchronized nature
>> of the
>> execute_task method. All in all, it seems to create a bottleneck
>> that is
>> going to be problematic, when the tasks are supposed to be ran
>> concurrently.
>>
>> Am I missing something ?
>>
>>
>> If more than one scheduled task could happen (which I doubt it
>> honestly) yes they would block on each other because the execute_task
>> function is synchronized. But we want to block here because the
>> delegated tasks MUST be executed in order,
>
> No, this is not required:
>
> "Multiple delegated tasks can be run in parallel."
> (https://docs.oracle.com/en/java/javase/17/docs/api/java.base/javax/net/ssl/SSLEngine.html#getDelegatedTask())
>
>
> Although it's not clear if they meant "multiple delegated tasks for
> different sessions" or "multiple delegated tasks for one session", I
> think the former is correct.
>
> The SSLEngine will manage the context, and any wrap/unwrap call will be
> blocked until all the tasks are executed.
>
> So my guess is that it makes sense to use another executor to process
> the tasks, such as in my proposal.
>
> Now, going back to the initial question - ie why do we need an executor
> when we execute a synchronized method - do you agree that my
> understanding is correct: this allow the IoProcessor thread to be freed
> for another session ?
>
>
> if a second scheduled task is created, it's a
>> guaranteed that any subsequent delegated tasks are actually executed
>> and not lost in the time between a previous execution of a delegated
>> task and the exit of the synchronized block in execute_task. We don't
>> want any weird race conditions here.
>
> I'm not sure we could face race condition when executing tasks. They are
> supposed to be safe from a concurrency PoV.
>
>
>>
>> I wanted to ensure that the async captured exception elevates to the
>> Filter exception handler in a very standard and known way.
>
> Now this is interesting. OTOH as soon as we catch an exception for any
> task being executed, we will process it. It *may* happen we have more
> than one, if we have more than one task being executed concurrently. But
> in this case, the session would just be shut down by the first exception
> being handled, no matter which one it is.
>
> A simple way
>> to force the handling of the exception on the filter-chain is to cause
>> a throwing of a special WriteRequest which will flow down the filter
>> chain and cause the error to be thrown. You already see where I'm
>> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L473
>> <https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L473>
>> writing the EncryptedWriteRequest from the async execute_task
>> function. Just create an EncryptedErrorRequest or some other object to
>> write and force the trigger.
>
> I'm not sure that is needed. Actually, the exception will pop up the
> chain, and the Handler will get it. What is important, and what is
> currently missing, is the alert that is not writen back to the remote
> peer (but that is the other thread's discussion).
>
>
>
> SSLEngine is a kind of nightmare...
>
--
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecharny@busit.com https://www.busit.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
Re: SSL handler and SSLEngine tasks
Posted by Emmanuel Lécharny <el...@gmail.com>.
On 02/03/2022 13:57, Jonathan Valliere wrote:
>
> CONFIDENTIALITY NOTICE: The contents of this email message and any
> attachments are intended solely for the addressee(s) and may contain
> confidential and/or privileged information and may be legally protected
> from disclosure.
>
>
> On Wed, Mar 2, 2022 at 7:33 AM Emmanuel Lécharny <elecharny@gmail.com
> <ma...@gmail.com>> wrote:
>
>
>
> On 02/03/2022 12:36, Jonathan Valliere wrote:
> > It’s already running in an Executor
>
> Can you clarify ? What is running in an executor?
>
>
> The SSLEngine delegated task is run in a thread pool executor outside of
> the filterchain.
> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L617
> <https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L617>
>
>
>
> which is why it’s a runnable
>
> Well, it's not related. You can create Runnable instances and execute
> them outside of an executor.
>
> . The
> > Runnable defers the actual execution logic to the execute task
> function
> > which should be the one that is looping to perform all queued
> tasks from
> > the SSL Engine.
>
> Yes, but my point is that we could spawn threads inside the execute
> task
> (one thread per task) instead of creating an executor inside the
> schedule_task.
>
>
> It uses a configured Executor instance which is generally the same one
> for every SSL session.
> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L84
> <https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L84>
> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLFilter.java#L72
> <https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLFilter.java#L72>
Ok, so we are on the same page here.
So the goal here is to spawn a thread that will process the
delegatedTask and let the IoProcessor deal with another session.
That's pretty much needed considering the potentially costly task done
in the delegated task.
>
>
> My fear is that bu creating a new thread for each schedule_task call is
> that we may have many pending threads waiting for the thread executing
> execute_task to be completed, because of the synchronized nature of the
> execute_task method. All in all, it seems to create a bottleneck
> that is
> going to be problematic, when the tasks are supposed to be ran
> concurrently.
>
> Am I missing something ?
>
>
> If more than one scheduled task could happen (which I doubt it honestly)
> yes they would block on each other because the execute_task function is
> synchronized. But we want to block here because the delegated tasks
> MUST be executed in order,
No, this is not required:
"Multiple delegated tasks can be run in parallel."
(https://docs.oracle.com/en/java/javase/17/docs/api/java.base/javax/net/ssl/SSLEngine.html#getDelegatedTask())
Although it's not clear if they meant "multiple delegated tasks for
different sessions" or "multiple delegated tasks for one session", I
think the former is correct.
The SSLEngine will manage the context, and any wrap/unwrap call will be
blocked until all the tasks are executed.
So my guess is that it makes sense to use another executor to process
the tasks, such as in my proposal.
Now, going back to the initial question - ie why do we need an executor
when we execute a synchronized method - do you agree that my
understanding is correct: this allow the IoProcessor thread to be freed
for another session ?
if a second scheduled task is created, it's a
> guaranteed that any subsequent delegated tasks are actually executed and
> not lost in the time between a previous execution of a delegated task
> and the exit of the synchronized block in execute_task. We don't want
> any weird race conditions here.
I'm not sure we could face race condition when executing tasks. They are
supposed to be safe from a concurrency PoV.
>
> I wanted to ensure that the async captured exception elevates to the
> Filter exception handler in a very standard and known way.
Now this is interesting. OTOH as soon as we catch an exception for any
task being executed, we will process it. It *may* happen we have more
than one, if we have more than one task being executed concurrently. But
in this case, the session would just be shut down by the first exception
being handled, no matter which one it is.
A simple way
> to force the handling of the exception on the filter-chain is to cause a
> throwing of a special WriteRequest which will flow down the filter chain
> and cause the error to be thrown. You already see where I'm
> https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L473
> <https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L473>
> writing the EncryptedWriteRequest from the async execute_task function.
> Just create an EncryptedErrorRequest or some other object to write and
> force the trigger.
I'm not sure that is needed. Actually, the exception will pop up the
chain, and the Handler will get it. What is important, and what is
currently missing, is the alert that is not writen back to the remote
peer (but that is the other thread's discussion).
SSLEngine is a kind of nightmare...
--
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecharny@busit.com https://www.busit.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
Re: SSL handler and SSLEngine tasks
Posted by Jonathan Valliere <jo...@emoten.com>.
CONFIDENTIALITY NOTICE: The contents of this email message and any
attachments are intended solely for the addressee(s) and may contain
confidential and/or privileged information and may be legally protected
from disclosure.
On Wed, Mar 2, 2022 at 7:33 AM Emmanuel Lécharny <el...@gmail.com>
wrote:
>
>
> On 02/03/2022 12:36, Jonathan Valliere wrote:
> > It’s already running in an Executor
>
> Can you clarify ? What is running in an executor?
>
The SSLEngine delegated task is run in a thread pool executor outside of
the filterchain.
https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L617
>
> which is why it’s a runnable
>
> Well, it's not related. You can create Runnable instances and execute
> them outside of an executor.
>
> . The
> > Runnable defers the actual execution logic to the execute task function
> > which should be the one that is looping to perform all queued tasks from
> > the SSL Engine.
>
> Yes, but my point is that we could spawn threads inside the execute task
> (one thread per task) instead of creating an executor inside the
> schedule_task.
>
It uses a configured Executor instance which is generally the same one for
every SSL session.
https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L84
https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLFilter.java#L72
>
> My fear is that bu creating a new thread for each schedule_task call is
> that we may have many pending threads waiting for the thread executing
> execute_task to be completed, because of the synchronized nature of the
> execute_task method. All in all, it seems to create a bottleneck that is
> going to be problematic, when the tasks are supposed to be ran
> concurrently.
>
> Am I missing something ?
>
If more than one scheduled task could happen (which I doubt it honestly)
yes they would block on each other because the execute_task function is
synchronized. But we want to block here because the delegated tasks MUST
be executed in order, if a second scheduled task is created, it's a
guaranteed that any subsequent delegated tasks are actually executed and
not lost in the time between a previous execution of a delegated task and
the exit of the synchronized block in execute_task. We don't want any
weird race conditions here.
I wanted to ensure that the async captured exception elevates to the Filter
exception handler in a very standard and known way. A simple way to force
the handling of the exception on the filter-chain is to cause a throwing of
a special WriteRequest which will flow down the filter chain and cause the
error to be thrown. You already see where I'm
https://github.com/apache/mina/blob/4fb5d0ee65c079efab08d66bd4c6897f76f39c4f/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L473
writing the EncryptedWriteRequest from the async execute_task function.
Just create an EncryptedErrorRequest or some other object to write and
force the trigger.
> >
> > On Wed, Mar 2, 2022 at 3:51 AM Emmanuel Lécharny <elecharny@gmail.com
> > <ma...@gmail.com>> wrote:
> >
> > Hi,
> >
> > an alternative would be to modify the execute_task method to become:
> >
> > synchronized protected void execute_task(NextFilter next) {
> > Runnable task = null;
> >
> > while ((task = mEngine.getDelegatedTask()) != null) {
> > try {
> > if (ENABLE_ASYNC_TASKS && (mExecutor != null)) {
> > mExecutor.execute(task);
> > } else {
> > task.run();
> > }
> >
> > write_handshake(next);
> > } catch (SSLException e) {
> >
> > So here, we push the tasks into an executor to be processed
> > concurrently.
> >
> > On 02/03/2022 07:05, Emmanuel Lécharny wrote:
> > > Hi Jonathan,
> > >
> > > I wonder if using an executor to process SSLEngine tasks is
> > useful at
> > > all, considering that the execute_task method is synchronized:
> > >
> > > protected void schedule_task(NextFilter next) {
> > > if (ENABLE_ASYNC_TASKS) {
> > > if (mExecutor == null) {
> > > execute_task(next);
> > > } else {
> > > mExecutor.execute(new Runnable() {
> > > @Override
> > > public void run() {
> > > SSLHandlerG0.this.execute_task(next);
> > > }
> > > });
> > >
> > > and
> > >
> > > synchronized protected void execute_task(NextFilter next) {
> > > ...
> > >
> > >
> > > Also the execute_task will pull tasks from the SSLEngine - we may
> > have
> > > more than one, so it will loop on the tasks - and it's unlikely
> > that we
> > > will have more tasks to execute when exiting the execute_tasks
> > method.
> > >
> > > The only benefit I can see is that the main thread will be freed
> for
> > > other tasks - like processing another IoSession - while tasks are
> > being
> > > processed by another thread. And may be this is what was
> > intended, but I
> > > wonder iff we should not simply spawn a dedicated thread for that
> > purpose.
> > >
> > > IMO, if we want to introduce an executor in the equation, it
> > should be
> > > done in the execute_task method, not in the schedule_task, it
> would
> > > speed up the TLS processing, while also freeing the main thread
> fast
> > > enough - even a bit slower than having the task loop being
> > processed by
> > > a dedicated thread.
> > >
> > > wdyt?
> > >
> >
> > --
> > *Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
> > T. +33 (0)4 89 97 36 50
> > P. +33 (0)6 08 33 32 61
> > emmanuel.lecharny@busit.com <ma...@busit.com>
> > https://www.busit.com/ <https://www.busit.com/>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
> > <ma...@mina.apache.org>
> > For additional commands, e-mail: dev-help@mina.apache.org
> > <ma...@mina.apache.org>
> >
> > --
> > CONFIDENTIALITY NOTICE: The contents of this email message and any
> > attachments are intended solely for the addressee(s) and may contain
> > confidential and/or privileged information and may be legally protected
> > from disclosure.
>
> --
> *Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
> T. +33 (0)4 89 97 36 50
> P. +33 (0)6 08 33 32 61
> emmanuel.lecharny@busit.com https://www.busit.com/
>
Re: SSL handler and SSLEngine tasks
Posted by Emmanuel Lécharny <el...@gmail.com>.
On 02/03/2022 12:36, Jonathan Valliere wrote:
> It’s already running in an Executor
Can you clarify ? What is running in an executor?
which is why it’s a runnable
Well, it's not related. You can create Runnable instances and execute
them outside of an executor.
. The
> Runnable defers the actual execution logic to the execute task function
> which should be the one that is looping to perform all queued tasks from
> the SSL Engine.
Yes, but my point is that we could spawn threads inside the execute task
(one thread per task) instead of creating an executor inside the
schedule_task.
My fear is that bu creating a new thread for each schedule_task call is
that we may have many pending threads waiting for the thread executing
execute_task to be completed, because of the synchronized nature of the
execute_task method. All in all, it seems to create a bottleneck that is
going to be problematic, when the tasks are supposed to be ran concurrently.
Am I missing something ?
>
> On Wed, Mar 2, 2022 at 3:51 AM Emmanuel Lécharny <elecharny@gmail.com
> <ma...@gmail.com>> wrote:
>
> Hi,
>
> an alternative would be to modify the execute_task method to become:
>
> synchronized protected void execute_task(NextFilter next) {
> Runnable task = null;
>
> while ((task = mEngine.getDelegatedTask()) != null) {
> try {
> if (ENABLE_ASYNC_TASKS && (mExecutor != null)) {
> mExecutor.execute(task);
> } else {
> task.run();
> }
>
> write_handshake(next);
> } catch (SSLException e) {
>
> So here, we push the tasks into an executor to be processed
> concurrently.
>
> On 02/03/2022 07:05, Emmanuel Lécharny wrote:
> > Hi Jonathan,
> >
> > I wonder if using an executor to process SSLEngine tasks is
> useful at
> > all, considering that the execute_task method is synchronized:
> >
> > protected void schedule_task(NextFilter next) {
> > if (ENABLE_ASYNC_TASKS) {
> > if (mExecutor == null) {
> > execute_task(next);
> > } else {
> > mExecutor.execute(new Runnable() {
> > @Override
> > public void run() {
> > SSLHandlerG0.this.execute_task(next);
> > }
> > });
> >
> > and
> >
> > synchronized protected void execute_task(NextFilter next) {
> > ...
> >
> >
> > Also the execute_task will pull tasks from the SSLEngine - we may
> have
> > more than one, so it will loop on the tasks - and it's unlikely
> that we
> > will have more tasks to execute when exiting the execute_tasks
> method.
> >
> > The only benefit I can see is that the main thread will be freed for
> > other tasks - like processing another IoSession - while tasks are
> being
> > processed by another thread. And may be this is what was
> intended, but I
> > wonder iff we should not simply spawn a dedicated thread for that
> purpose.
> >
> > IMO, if we want to introduce an executor in the equation, it
> should be
> > done in the execute_task method, not in the schedule_task, it would
> > speed up the TLS processing, while also freeing the main thread fast
> > enough - even a bit slower than having the task loop being
> processed by
> > a dedicated thread.
> >
> > wdyt?
> >
>
> --
> *Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
> T. +33 (0)4 89 97 36 50
> P. +33 (0)6 08 33 32 61
> emmanuel.lecharny@busit.com <ma...@busit.com>
> https://www.busit.com/ <https://www.busit.com/>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
> <ma...@mina.apache.org>
> For additional commands, e-mail: dev-help@mina.apache.org
> <ma...@mina.apache.org>
>
> --
> CONFIDENTIALITY NOTICE: The contents of this email message and any
> attachments are intended solely for the addressee(s) and may contain
> confidential and/or privileged information and may be legally protected
> from disclosure.
--
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecharny@busit.com https://www.busit.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
Re: SSL handler and SSLEngine tasks
Posted by Jonathan Valliere <jo...@emoten.com>.
It’s already running in an Executor which is why it’s a runnable. The
Runnable defers the actual execution logic to the execute task function
which should be the one that is looping to perform all queued tasks from
the SSL Engine.
On Wed, Mar 2, 2022 at 3:51 AM Emmanuel Lécharny <el...@gmail.com>
wrote:
> Hi,
>
> an alternative would be to modify the execute_task method to become:
>
> synchronized protected void execute_task(NextFilter next) {
> Runnable task = null;
>
> while ((task = mEngine.getDelegatedTask()) != null) {
> try {
> if (ENABLE_ASYNC_TASKS && (mExecutor != null)) {
> mExecutor.execute(task);
> } else {
> task.run();
> }
>
> write_handshake(next);
> } catch (SSLException e) {
>
> So here, we push the tasks into an executor to be processed concurrently.
>
> On 02/03/2022 07:05, Emmanuel Lécharny wrote:
> > Hi Jonathan,
> >
> > I wonder if using an executor to process SSLEngine tasks is useful at
> > all, considering that the execute_task method is synchronized:
> >
> > protected void schedule_task(NextFilter next) {
> > if (ENABLE_ASYNC_TASKS) {
> > if (mExecutor == null) {
> > execute_task(next);
> > } else {
> > mExecutor.execute(new Runnable() {
> > @Override
> > public void run() {
> > SSLHandlerG0.this.execute_task(next);
> > }
> > });
> >
> > and
> >
> > synchronized protected void execute_task(NextFilter next) {
> > ...
> >
> >
> > Also the execute_task will pull tasks from the SSLEngine - we may have
> > more than one, so it will loop on the tasks - and it's unlikely that we
> > will have more tasks to execute when exiting the execute_tasks method.
> >
> > The only benefit I can see is that the main thread will be freed for
> > other tasks - like processing another IoSession - while tasks are being
> > processed by another thread. And may be this is what was intended, but I
> > wonder iff we should not simply spawn a dedicated thread for that
> purpose.
> >
> > IMO, if we want to introduce an executor in the equation, it should be
> > done in the execute_task method, not in the schedule_task, it would
> > speed up the TLS processing, while also freeing the main thread fast
> > enough - even a bit slower than having the task loop being processed by
> > a dedicated thread.
> >
> > wdyt?
> >
>
> --
> *Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
> T. +33 (0)4 89 97 36 50
> P. +33 (0)6 08 33 32 61
> emmanuel.lecharny@busit.com https://www.busit.com/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
> For additional commands, e-mail: dev-help@mina.apache.org
>
> --
CONFIDENTIALITY NOTICE: The contents of this email message and any
attachments are intended solely for the addressee(s) and may contain
confidential and/or privileged information and may be legally protected
from disclosure.
Re: SSL handler and SSLEngine tasks
Posted by Emmanuel Lécharny <el...@gmail.com>.
Hi,
an alternative would be to modify the execute_task method to become:
synchronized protected void execute_task(NextFilter next) {
Runnable task = null;
while ((task = mEngine.getDelegatedTask()) != null) {
try {
if (ENABLE_ASYNC_TASKS && (mExecutor != null)) {
mExecutor.execute(task);
} else {
task.run();
}
write_handshake(next);
} catch (SSLException e) {
So here, we push the tasks into an executor to be processed concurrently.
On 02/03/2022 07:05, Emmanuel Lécharny wrote:
> Hi Jonathan,
>
> I wonder if using an executor to process SSLEngine tasks is useful at
> all, considering that the execute_task method is synchronized:
>
> protected void schedule_task(NextFilter next) {
> if (ENABLE_ASYNC_TASKS) {
> if (mExecutor == null) {
> execute_task(next);
> } else {
> mExecutor.execute(new Runnable() {
> @Override
> public void run() {
> SSLHandlerG0.this.execute_task(next);
> }
> });
>
> and
>
> synchronized protected void execute_task(NextFilter next) {
> ...
>
>
> Also the execute_task will pull tasks from the SSLEngine - we may have
> more than one, so it will loop on the tasks - and it's unlikely that we
> will have more tasks to execute when exiting the execute_tasks method.
>
> The only benefit I can see is that the main thread will be freed for
> other tasks - like processing another IoSession - while tasks are being
> processed by another thread. And may be this is what was intended, but I
> wonder iff we should not simply spawn a dedicated thread for that purpose.
>
> IMO, if we want to introduce an executor in the equation, it should be
> done in the execute_task method, not in the schedule_task, it would
> speed up the TLS processing, while also freeing the main thread fast
> enough - even a bit slower than having the task loop being processed by
> a dedicated thread.
>
> wdyt?
>
--
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecharny@busit.com https://www.busit.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org