Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't process close() until current in-flight async ops are complete #32

Closed
rvagg opened this issue Apr 8, 2013 · 9 comments · Fixed by #612
Closed

Don't process close() until current in-flight async ops are complete #32

rvagg opened this issue Apr 8, 2013 · 9 comments · Fixed by #612
Assignees
Labels
enhancement New feature or request semver-patch Bug fixes that are backward compatible
Milestone

Comments

@rvagg
Copy link
Member

rvagg commented Apr 8, 2013

Will involve more tracking like the delete-iterators-on-close work. Each async operation will have to increment a counter when starting and decrement when done, will also need to trigger a possible-close function when done.
Without this, a segfault is possible when you close() while an operation is in-flight.

@dominictarr
Copy link
Contributor

Is this only iterators, or put, get, del, batch also?

@mcollina
Copy link
Member

mcollina commented May 1, 2013

Even adding a counter is hard, because after a call to close, all the subsequent accesses to the database must be ignored.

As I see, every lib implements this thing on its own. Could we build a generic and reusable solution? Even communication protocols have the same problem if they want to close gracefully.

@rvagg
Copy link
Member Author

rvagg commented May 12, 2014

marked as "help wanted", see the iterator tracking functionality for how this could work since we hold up close() until iterators are closed and can also force close of iterators when close() is called.

@ralphtheninja
Copy link
Member

@dominictarr I believe it involves all in flight operations (see #157)

@ralphtheninja
Copy link
Member

@mcollina The counter would have to be static I guess.

@ralphtheninja ralphtheninja self-assigned this May 17, 2015
@ralphtheninja ralphtheninja removed their assignment Oct 14, 2016
@martinheidegger
Copy link

Shouldn't this be a feature of levelup, I mean: It is sort of a pain to do this for every client.

@ralphtheninja
Copy link
Member

Or maybe in abstract-leveldown?

@martinheidegger
Copy link

martinheidegger commented Oct 18, 2016

In any case https://github.com/Level/levelup#dbclosecallback doesn't mention this case and at the minimum it should state what the should-case should be.

@vweevers vweevers added the enhancement New feature or request label Dec 31, 2018
@vweevers vweevers self-assigned this Mar 9, 2019
@vweevers vweevers removed the help wanted Extra attention is needed label Mar 9, 2019
@vweevers vweevers added this to the 5.1.0 milestone Mar 25, 2019
@vweevers vweevers pinned this issue Mar 31, 2019
@vweevers vweevers added the semver-patch Bug fixes that are backward compatible label Mar 31, 2019
@ralphtheninja
Copy link
Member

Nice!

@vweevers vweevers unpinned this issue Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request semver-patch Bug fixes that are backward compatible
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants