a deadlock will occur when new a Task and start it without specify TaskScheduler

[inversionhourglass]

DotNetty uses the ExecutorTaskScheduler as the default TaskScheduler, and it makes TaskScheduler.Current become ExecutorTaskScheduler. And the task.Start method with no-parameter uses TaskScheduler.Current as default value. So, when we create a task and start it without specify TaskScheduler, the task will enqueue and wait for request processing code finish, if we wait for result after task start, a deadlock will occur.

We can easily test that, just modify HelloServerHandler.Process from HttpServer example:

void Process(IChannelHandlerContext ctx, IHttpRequest request)
{
    string uri = request.Uri;
    switch (uri)
    {
        case "/plaintext":
            this.WriteResponse(ctx, PlaintextContentBuffer.Duplicate(), TypePlain, PlaintextClheaderValue);
            break;
        case "/json":
            var t = new System.Threading.Tasks.Task(() => { });
            t.Start();
            t.Wait(); // deadlock occur
            byte[] json = Encoding.UTF8.GetBytes(NewMessage().ToJsonFormat());
            this.WriteResponse(ctx, Unpooled.WrappedBuffer(json), TypeJson, JsonClheaderValue);
            break;
        default:
            var response = new DefaultFullHttpResponse(HttpVersion.Http11, HttpResponseStatus.NotFound, Unpooled.Empty, false);
            ctx.WriteAndFlushAsync(response);
            ctx.CloseAsync();
            break;
    }
}

Of course, we can specify TaskScheduler.Default as TaskScheduler to start task, but some third-party nuget we can not control. Any way to figure out this problem?

[inversionhourglass]

491

[caozhiyuan]

you can use task factory startnew and pass taskscheduler default .

[yyjdelete]

Maybe ＃491 should be an option(default=disable), since it's an huge break change. Some user like me expected anything after await still happen on the same thread for dotnetty thread, and do no thread-safe on for the same connection.

And by the way, call Wait() on an Task is always an possible deadlock on WinForm and Asp.Net(both not for netcore, all of them use SynchronizationContext but DotNetty use TaskScheduler) when use await without ConfigureAwait(false). Task.Run(() => AnotherAsync()).Wait(); or Task<T>.Run(() => AnotherAsync()).Result; maybe helpful. And

async void Process(IChannelHandlerContext ctx, IHttpRequest request)
{
    try
    {
        //DotNetty Loop Thread 1
        await Task.Delay(100);//Or any async call
        //Expected still in DotNetty Loop Thread 1
    } catch {
        //Any exception in async void will crash the whole program without catch it
    }
}

[inversionhourglass]

@caozhiyuan @yyjdelete 老哥们，我看你们都是国人，我英语不好，我直接中文了。 @caozhiyuan 使用TaskFactory指定TaskScheduler或者在Task.Start的时候指定TaskScheduler是都可以，但是我们不能保证第三方库内部都指定了TaskScheduler，现在我就是在用ES的时候死锁了，所以最好的方式还是修改DotNetty，让Task的写法不受限制。我提交的#491 PullRequest是我想到的一种方士。 @yyjdelete 你前面说的WinForm和Asp.Net的死锁我明白，后面的那段代码啥意思，Process方法改成异步的？

[yyjdelete]

@inversionhourglass 最后那段就是 #265 一样的, #491 如果不是一个可选加入的选项的话, 可能会破坏掉现在已有的使用DotNetty的异步用户代码, 因为现在await前后的代码不是在同一线程中执行的, 可能会导致后续的所有代码都出现线程不安全的问题.

另外刚试了下, 你可以通过添加一个新函数来包裹现在的函数Process->Process0(不过这个lambda的闭包会多一次内存分配, 不知道还有没有其他办法), 在用户代码中来隐藏DotNetty的TaskScheduler, Process0的执行依然是同步的(在之前DotNetty的线程中), 但不会发生死锁.

---

Anyone who need this can also wrap user code with the below code to disable the TaskScheduler of DotNetty. the code will run in sync, just with a little more overcost for lambda capture.

void Process(IChannelHandlerContext ctx, IHttpRequest request)
{
        new Task(() => Process0(ctx, request), TaskCreationOptions.HideScheduler).RunSynchronously();
}

[caozhiyuan]

block netty thread is bad idea, business logic can run in threadpool（input netty context state ）.

like this : https://github.com/caozhiyuan/DotNetty/blob/dev/src/DotNetty.Rpc/Server/RpcHandler.cs#L28

[inversionhourglass]

@yyjdelete 正如 @caozhiyuan 所说，我觉得 #265 里面最后的那个建议不是一个好的建议。

protected override void ChannelRead(IChannelHandlerContext ctx, object message) {
    ctx.WriteAndFlushAsync(resp).Wait();
    _redisClient.SetGatewayEndpointAsync(authDevice.DeviceId.ToString(), _protocolGatewayEndPoint).Wait();
});

这种方式虽然能强制遵守DotNetty的线程规范（同一个连接的所有请求都在同一个EventLoop中执行，并且是串行执行），但是这会阻塞netty的线程就让异步编程失去了意义，异步编程本身就是为了在代码执行IO操作的时候线程池线程能够释放去执行其他任务，这样能更高效的使用线程，而且netty线程默认是CPU逻辑核心数*2，这个数量并不多，如果某一时间突然遇到几个慢IO问题，那可能会导致服务完全阻塞。

265 中第一种写法将ChannelRead改为async void，这种方式我尝试过，也会发生死锁，#265 中也有提到。

再就是使用Task包裹一层的写法：

protected override void ChannelRead(IChannelHandlerContext ctx, object message) {
    var t = Task.Run(async () => {
        await ctx.WriteAndFlushAsync(resp).
        await _redisClient.SetGatewayEndpointAsync(authDevice.DeviceId.ToString(), _protocolGatewayEndPoint)
    });
    t.Wait();
});

上面的代码也分两种方式，一直是对Task进行Wait，另一种不Wait。不Wait就是你所说的没有遵守DotNetty的代码规范，会线程不安全（其实不会）。其实不论是否Wait，我认为都不是一种好的方式（Wait就更糟糕）。在不调用Wait方法的情况下，这种写法使得Netty的线程模型完全失去了意义，因为实际上你所有的请求最终还是直接交给了线程池来处理。而如果调用Wait，那除了实际使用线程池处理外，还会阻塞Netty线程，这问题就非常大了。 在我看来，上面的代码应该修改为：

protected override void ChannelRead(IChannelHandlerContext ctx, object message) {
    ctx.WriteAndFlushAsync(resp).ContinueWith(t => {
        _redisClient.SetGatewayEndpointAsync(authDevice.DeviceId.ToString(), _protocolGatewayEndPoint);
    });
});

或者

protected override void ChannelRead(IChannelHandlerContext ctx, object message) {
    ProcessAsync(ctx, line);
});

private async Task ProcessAsync(IChannelHandlerContext ctx, object message) {
    await ctx.WriteAndFlushAsync(resp);
    await _redisClient.SetGatewayEndpointAsync(authDevice.DeviceId.ToString(), _protocolGatewayEndPoint);
    });
});

遇IO操作异步之后的处理交由线程池处理，让netty线程不必等待。 这种方式只需要限定两个条件就不存在并发不安全的问题： 1.服务端需同步从IByteBuffer中提取一条完整请求 2.保证调用方同一个连接发送一个请求后需要等待请求返回或超时才能发送第二个请求

我之所以认为这样不会存在线程不安全的问题，是因为在我看来，netty只需要保证连接的建立、数据接收、连接关闭以及其他一些关键事件在同一个EventLoop中保持顺序即可。netty会将数据读取到IByteBuffer中，同步读取是为了避免拆包的情况下，并发读取导致每次并发获取到的都是请求片段无法获取完整请求，最终导致无法提取请求。而第二点是为了避免服务端并发处理同一个连接发送的多个请求，然后并发返回数据，这时候并发返回写入数据会导致数据乱序，调用方无法读取数据。当然如果调用方只发，服务端只收，也不需要保证第二点。

如果我这方方式没有问题，那 #491 的修复方式理论是可行的。

[inversionhourglass]

@caozhiyuan https://github.com/caozhiyuan/DotNetty/blob/dev/src/DotNetty.Rpc/Server/RpcHandler.cs#L28 this solution can change TaskScheduler to default and will not block the netty thread, but it just looks like receive request and put it into ThreadPool, if just do like this, why need child group eventloop? why not just put it into ThreadPooll when received request. https://github.com/Azure/DotNetty/blob/4a76e624580e0005257513fce1c69555294763f5/src/DotNetty.Transport/Channels/Sockets/AbstractSocketChannel.cs#L199-L208

In my opinion, it should execute synchronously until the IO operation occurs.

[caozhiyuan]

if do socket connect , eventLoop.InEventLoop just make sure next event run in eventLoop. eventloop only use for network and netty self event. you can compare it with some java netty component.

@inversionhourglass

[yyjdelete]

@inversionhourglass 没有看明白https://github.com/Azure/DotNetty/issues/490#issuecomment-494640361, 但 #491 的调整如果不是可选加入的话, 例如下面一段代码(只是个例子示意), 之前对于每个连接一个Handler的情况是能正常工作的(await 后的延续将仍被调度给EventLoop, 在之前线程上发生因此不用考虑线程安全: recv1, recv2, i++1, recv3, i++2,i++3,...). 而修改之后不行, 因为await 后的延续被调度给了TaskScheduler.Default, 后续的延续将在随机线程池线程上发生, 而i++并不是线程安全的, 在i++同时发生在不同线程池线程上的时候会得到错误的结构

        public class XXHandler : SimpleChannelInboundHandler<IByteBuffer>
        {
            public int i;
            protected override async void ChannelRead0(IChannelHandlerContext ctx, IByteBuffer msg)
            {
                try
                {
                    await System.Threading.Tasks.Task.Delay(100);
                    i++;
                }
                catch (Exception e)
                {
                    ExceptionCaught(ctx, e);
                }
            }
        }

的确Task.Run比较浪费线程池, 在 https://github.com/Azure/DotNetty/issues/490#issuecomment-494443066 中提供了另外一种方式来对于第三方库或者无需此特性的代码局部禁用DotNetty的TaskScheduler(用到了Task, 但事实上是同步执行的)

至于纯粹的await/async(没有.Wait()/.Result()/.GetAwaiter().GetResult())产生死锁的情况目前还没有遇到过(也可能遇到了但没记), 等遇到了再看看(懒得为了这个去装一个redis)

[inversionhourglass]

@yyjdelete 我明白你说的这个并发问题了，不过只要在从IByteBuffer中同步提取完整数据后做无状态操作（也就是不与全局或连接共享数据交互）就没问题，如果一定要交互，那就只能加锁或原子处理了。netty线程有限，相对于阻塞等待，加锁的代价要小一些

[inversionhourglass]

@caozhiyuan okay, getting time I will look into java netty component. I haven't seen it yet. Thanks for your reply.

[caozhiyuan]

@inversionhourglass yes, such as dubbo , nettyrpc and so on.

[StayGoldTY]

我现在有一个类似的问题，在不同的线程里面申请内存和释放内存导致就算调用了WriteAndFlushAsync，其实内存还是没有释放。 因为task.run的方式可以不必等待，而用上面阻塞的方式的确可以保证不发生内存泄露，但是程序性能明显慢了很多，不知道楼主有解决办法吗

[inversionhourglass]

@StayGoldTY 我之前的问题已经记不清了，你描述的这个问题理论也不会导致内存泄漏，只是WriteAndFlushAsync异步处理可能存在一些滞后，如果是内存泄漏的问题，可能需要从别的方面再看看了

[StayGoldTY]

我自己通过反复测试只要不用task.run或者task.factory 这种多线程而用awiat的写法，就没有内存泄露，而且通过对每个buffer进行标记，可以确定泄露的buffer就是WriteAndFlushAsync要发送的到客户端的buffer。而且虽然线程不一样，我尽量保证在同一个eventloop里面来执行了，没想到还是泄露。我看了之前java那边的netty4最开始的版本好像有这个问题，后面好像修复了。dotnetty这边不知道底层是哪个地方影响了一直还有这个问题。

[inversionhourglass]

你加油，找到原因了可以来这里分享一下