Skip to content

Prepared: Prevent mem allocs during cleanup #138

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

Merged
merged 1 commit into from
Oct 20, 2017

Conversation

Marenz
Copy link

@Marenz Marenz commented Oct 12, 2017

If a prepared statement is collected during a GC run and the d'tor is
called, it causes another allocation through either:

  • allocation of the packet array
  • allocation of an exception when trying to write to a closed connection

In both cases, the GC freaks out and throws an InvalidMemoryOperation.

This commit fixes the two allocation cases I found. There may be more.

Here is an example backtrace of this:
https://gist.github.com/Marenz/b3293f3a6daa9706a5552a4e68b1cec5

If a prepared statement is collected during a GC run and the d'tor is
called, it causes another allocation through either:

- allocation of the packet array
- allocation of an exception when trying to write to a closed connection

In both cases, the GC freaks out and throws an InvalidMemoryOperation.

This commit fixes the two allocation cases I found. There may be more.
@Marenz
Copy link
Author

Marenz commented Oct 12, 2017

@Abscissa it would be cool if you could release this as a new version, too. I am hitting that bug very easy in my project

@Marenz
Copy link
Author

Marenz commented Oct 18, 2017

Ping?

@Abscissa Abscissa merged commit 74cf90a into mysql-d:master Oct 20, 2017
@Abscissa
Copy link

Sorry, been busy. PR looks good, merging.

New release is being held up by one thing: recent #132/#135 discussion revealed some build failures on certain compiler versions which I need to take care of (mainly just working around https://issues.dlang.org/show_bug.cgi?id=17482 IIRC).

@Marenz Marenz deleted the fix-invalid-mem-op branch October 20, 2017 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants