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

Using GC to allocate event driver results in trailing segfault #90

Closed
schveiguy opened this issue Nov 2, 2018 · 6 comments
Closed

Using GC to allocate event driver results in trailing segfault #90

schveiguy opened this issue Nov 2, 2018 · 6 comments

Comments

@schveiguy
Copy link

Ever since I upgraded to vibe.d v 0.8.4, I get a lingering segfault when my app is shutting down. I've traced it down to mysqlnative trying to close a TCPConnection, and the TCPConnection's context pointer being invalid memory.

Looking at TCPConnection, the only way it could have this happen is if the context data is deallocated underneath it. I think the issue is that because the event driver is allocated using "new", and therefore by the GC, the order of destruction could be the event driver first and then sockets inside other classes second. In this case, the event driver's context is freed before the TCPConnection can use it to figure out how to release the resources it has.

The reason it's coming from mysqlnative is because it allocates connections using the GC.

In order to do this correctly, the event driver has to be allocated by C malloc, and must be freed only after all shutdown happens (including the GC). This is a tough one, and perhaps the only fix really is to allocate it and never free it.

@schveiguy
Copy link
Author

ping @Abscissa this may be something you've experienced.

@schveiguy
Copy link
Author

schveiguy commented Nov 2, 2018

Hm... I take it back, it actually gets freed synchronously, but BEFORE the final GC. So it's guaranteed to segfault.

Like I said, I'm not sure exactly how to solve it. Perhaps you can check if any sockets/contexts are open, and if so, don't free it (and use C malloc to allocate the driver).

@schveiguy
Copy link
Author

I'm going to work on a potential PR to fix this. I think it actually can be done without leaving any lingering resources -- you just have to defer destruction of the class until all the resources are released. Still have to allocate on the C heap though.

@schveiguy
Copy link
Author

OMG, I just realized, this was fixed by @s-ludwig 11 days ago in #87. I've been looking at my local download of eventcore debugging this. So hey, thanks for fixing it before I even discovered it, that's some service! 😜

@schveiguy
Copy link
Author

schveiguy commented Nov 6, 2018

BTW, for the PR I was going to write, I had conceived of having the release resources function calling dispose automatically if dispose was already requested, but delayed due to still having open FDs. You'd just need a callback of some kind in the drivers that need it.

@s-ludwig
Copy link
Member

s-ludwig commented Nov 6, 2018

I was originally going to implement it that way, too, but figured that since the application is usually shut down anyway, and the case where just a thread is shut down is more difficult to support correctly, it was better to just emit a warning and leave the OS clean up the rest. Emitting a warning and shutting down the driver lazily would of course have been better, but the gain/effort ratio currently was just not high enough for me.

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

No branches or pull requests

2 participants