Resolve Outbound Deadlock Issue

[kingster]

The deadlock used to happen due to the clientHandler.onConnect handler being run on a sync thread. Hence any sync method would get blocked. AsyncApi still used to work, but the example provided was broken. With onConnect running on a separate thread the netty handler is released for further processing of responses which would complete the pending future.

Resolve #13

[livem]

You can change the callbackExecutor with the method setCallbackExecutor outside whatever you want, without modify the OutboundChannelInitializer inside.

[kingster]

I wanted to modify the default callbackExecutor from SingleThreadExecutor to FixedThreadPool(2) for default execution by OutboundClientHandler, one being for handling ESL events and another for command-response. This is default behavior change hence changed here. Someone can still override and set it to much larger pool based on use-case by calling the setCallbackExecutor.

Incase you think this should be still newSingleThreadExecutor and OutboundClientHandler should all times override this, then I can make that change.

[ykalayy]

@kingster Whenever you created a different EventExecutorGroup for the handler you don't need to make it two for the command-response messages. It will be handled by EventExecutorGroup thread

[livem]

A thread pool with 2 thread, one being for handling ESL events and another for command-response. That was a gread idea. On 6/4/2019 17:14，Kinshuk Bairagi<notifications@github.com> wrote： @kingster commented on this pull request. In src/main/java/org/freeswitch/esl/client/outbound/OutboundChannelInitializer.java:

@@ -12,7 +14,8 @@

public class OutboundChannelInitializer extends ChannelInitializer<SocketChannel> { private final IClientHandlerFactory clientHandlerFactory; - private ExecutorService callbackExecutor = Executors.newSingleThreadExecutor(); + private ExecutorService callbackExecutor = Executors.newFixedThreadPool(2); I wanted to modify the default callbackExecutor from SingleThreadExecutor to FixedThreadPool(2) for default execution by OutboundClientHandler, one being for handling ESL events and another for command-response. This is default behavior change hence changed here. Someone can still override and set it to much larger pool based on use-case by calling the setCallbackExecutor. Incase you think this should be still newSingleThreadExecutor and OutboundClientHandler should all times override this, then I can make that change. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[livem]

Can you offer some Outbound Deadlock case ?
Some example code at FreeSWITCH side ?

<action application="socket" data="127.0.0.1:8084"/>

or

<action application="socket" data="127.0.0.1:8084 async"/> 

[kingster]

@livem This is what I am using at fs side

 <action application="socket" data="127.0.0.1:8084 async full"/>

[phyoo]

@livem This is what I am using at fs side

 <action application="socket" data="127.0.0.1:8084 async full"/>

I have changed the code just like you，and I am using it like

OutboundTest.java

and I am using it at fs side just like you too，but outbound still doesn't work.This is the fs's log:

2019-06-26 16:48:50.317129 [ERR] mod_event_socket.c:481 Socket Error: Connection refused
2019-06-26 16:48:50.317129 [ERR] mod_event_socket.c:485 Socket Error!

but when I am using

nc -l -k localhost 8084

it works.
any help?

[kingster]

A Socket Error: Connection refused error would mean that the ESL destination is unreachable, please check your system firewall policies to allow the port. A localhost is a loopback address hence reachable directly

[ykalayy]

@kingster
Seems this repo has been abandoned. I forked and fixed that memory leak issue & outbound deadlock issue with EvenLoopGroudExecutor. You can use my repository.
Welcome to contribute
https://github.com/ykalayy/esl-client