Deno: Closer should return a Promise

Created on 19 May 2020  路  3Comments  路  Source: denoland/deno

Deno Conns provide a Reader, Writer, and Closer. The first two are promise driven apis, the last one is not.

There are possibly many async operations (one trying to read, and many other async writes) in an active connection.
When calling close() on the connection, it is possible that there are many pending writer ops, enough that any simple test reading/writing will fail with resource leakage (thanks for this).

This means that in order to close in a somewhat clean way, all API clients will have to set up a promise that resolves on a read failure (similar issue on listen) and track the individual promises returned by write into a stack and shift() them out as they resolve.

The setup to call close() typically means setting up a flag to ignore any read/write attempts from some other async operation trying to use the connection, and then waiting on the reader promise and all the write promises to resolve/fail. Only then can close() be called.

Since the Conn should know about the state of the read and the number of pending writes scheduled, it seems reasonable to prevent much of this boilerplate by simply returning a promise from close(), which would signal at that point when the controlling process can safely exit or whatnot.

cli docs

Most helpful comment

FYI there firmly won't be anymore breaking API changes in the Deno namespace: https://deno.land/v1#stability. I think this is purposefully sync because there _ought_ to be a way to close anything synchronously.

This means that in order to close in a somewhat clean way, all API clients will have to set up a promise that resolves on a read failure (similar issue on listen) and track the individual promises returned by write into a stack and shift() them out as they resolve.

Sounds like Promise.all() would help.

All 3 comments

FYI there firmly won't be anymore breaking API changes in the Deno namespace: https://deno.land/v1#stability. I think this is purposefully sync because there _ought_ to be a way to close anything synchronously.

This means that in order to close in a somewhat clean way, all API clients will have to set up a promise that resolves on a read failure (similar issue on listen) and track the individual promises returned by write into a stack and shift() them out as they resolve.

Sounds like Promise.all() would help.

I think this is purposefully sync because there ought to be a way to close anything synchronously.

More specifically close signals actually closing the resource and no longer working on it. It should not attempt to complete pending operations - working with it after calling .close is invalid.

If you want to wait for pending operations wait for them explicitly and only then .close.

I think there should probably be a doc change explaining why close doesn't return a promise.

Sounds like Promise.all() would help.

Actually it won't because multiple async write operations are not thread safe - the underlying rust can interleave the writes.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

doutchnugget picture doutchnugget  路  3Comments

benjamingr picture benjamingr  路  3Comments

watilde picture watilde  路  3Comments

CruxCv picture CruxCv  路  3Comments

somombo picture somombo  路  3Comments