@lidizheng

del of the Python Channel class. Turned out, the Python Channel object is short-lived and might be deallocated right away in cases like stub(grpc.insecure_channel(...));

That doesn't make sense to me. Stubs need to retain a reference to their channel, otherwise they won't be able to actually send an RPC.

That doesn't make sense to me. Stubs need to retain a reference to their channel, otherwise they won't be able to actually send an RPC.

It occurs when we have a RPC lives longer than both the stub and the channel. Before the call is finished, both the stub and Python channel objects are deallocated, but the Cython channel object and couple "state" objects stays. The reference between states is a bit messy, but they are keeps essential objects alive.

def fire_and_forget():
    channel = grpc.insecure_channel(...)
    stub = TestStub(channel)
    call = stub.Call(...)
    call.add_done_callback(logic)

for i in range(1000):
    fire_and_forget()

@gnossen

When I was writing the example, I wonder what should happen if users use with clause but fires RPC using future or invoked add_done_callback?

with grpc.insecure_channel(...) as channel:
    stub = TestStub(channel)
    call = stub.Call(...)
    call.add_done_callback(logic)

Should the call falls immediately? Or should the channel shutdown gracefully (no new RPCs but allow ongoing ones to continue)? WDYT?

I don't think we should encourage any usage that doesn't use the context manager form or an explicit close. The "fire and forget" example is a bit contrived. If someone actually came to us with this question, I would suggest that the example be rewritten to:

def fire(stub):
    call = stub.Call(...)
    call.add_done_callback(logic)

with grpc.insecure_channel(...) as channel:
  stub = TestStub(channel)
  for i in range(1000):
      fire(stub)
  await_all_callbacks_done()

But, viewing this as a corner case, it would be surprising to me if my callback failed to execute. So I would say I'd prefer a "graceful shutdown".

Edit: Added an all-important synchronization step before closing the channel

In current implementation, the close of channel immediately cancels all ongoing RPC...

That's true, which isn't ideal. But again, I view this as a corner case. Not many people see this rough edge because:

1. Most people use the synchronous API.
2. We recommend sharing a single channel across many RPCs.

@lidizheng which release is this code going to be in? I see that it is merged, but it seems that master does not correspond to the latest release as far as i can tell from the release notes

@szikanova We have daily release for our master branch: https://packages.grpc.io/. The interval of releases is around 6 weeks.

@lidizheng thanks! I noticed that the last daily release seems to be May 15, is that expected?

@szikanova You remind me that there is a ongoing failure blocking our build automation. See the tracker issue #23047. This PR should be included in May 15's build.

After upgrade to 1.30.0, we have started to experience exception upon VM exit with stack trace like:

  File "/usr/local/lib/python3.6/dist-packages/grpc/_channel.py", line 1126, in __del__
  File "src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi", line 515, in grpc._cython.cygrpc.Channel.close
  File "src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi", line 399, in grpc._cython.cygrpc._close
  File "src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi", line 429, in grpc._cython.cygrpc._close
  File "/usr/lib/python3.6/threading.py", line 364, in notify_all
  File "/usr/lib/python3.6/threading.py", line 347, in notify

It seems either _deque or _islice has been garbaged collected. Found a pretty old issue here.

This doesn't do any harm but just noisy exception.

@honnix I guess we will be better off suppressing the exception log. Thanks for the info. Drafting a fix in #23351.

@lidizheng Thanks for looking into this and a quick fix.