autoPurge can throw an exception if the socket was closed without purging #154
Comments
|
I am "simulating" the error by restarting the mysql server after a connection is connected. I tried adding a check to see if the socket is open and returning from autoPurge before trying anything, but now instead of crashing, the server gets stuck. So that's not the only answer. Still investigating... I would recommend adding a test to your test suite that restarts the mysql server while a connection is connected. This definitely can happen in practice. |
|
More info: the call is failing I think due to already-buffered data. Still investigating. |
|
Just a couple off-the-top-of-my-head ideas (don't have time to dig into this right now): I wonder if maybe Connection's _open member is partly at fault here and should be eliminated in favor of directly asking the Socket if it's open (which, it appears Connection.closed() already does, but looks like there are some places that access Also, the abstraction over Phobos sockets has a scope guard that closes the socket upon failure, but I notice the equivalent Vibe socket code lacks that. Maybe related to what you're experiencing? |
|
OK, I figured out the issue. As I suspected, when My attempt to fix the problem was to just return immediately from The reason I thought it may be already buffered data is because I stopped waiting for a response in my browser, and tried sending another request. For some reason it gets the "server packet out of order" error. I printed out the packet numbers and it got 31, but was expecting 1. I'm not sure how this happened, and it's definitely not a buffering issue, as the new connection is made by creating an entirely new object. Perhaps it's getting this same connection, and something weird is happening. It shouldn't happen that way, but I'm not sure what that problem is at this point. I'll just chalk it up to "I have no fucking clue what I was doing" ;) I have since fixed figured out how to reset the state, so now I have |
|
One minor addition: I'll submit the PR, but I'm not 100% sure how the Phobos socket works, as I'm using vibe.d. So you should test the PR against that use case (simulate by restarting the mysql server). |
|
On January 25, 2018 7:31:40 PM EST, Steven Schveighoffer ***@***.***> wrote:
One minor addition: I'll submit the PR, but I'm not 100% sure how the
Phobos socket works, as I'm using vibe.d. So you should test the PR
against that use case (simulate by restarting the mysql server).
Great. Thanks for working this one out.
The travis tester is set up to run the './run-tests' script which automatically runs all tests using both phobos and vibe sockets. So that part shouldnt be an issue.
Can the problem be simulated by bypassing Connection.close and directly closing the socket itself, behind mysqln's back? (Or something else like that?) That would make it easier to create test cases for this. And itd be nice, if at all possible, to not have to have any tests relying on what tend to be system-specific commands for restarting services/daemons, or to need to disrupt a server that may already be in use for other things. Of course, this wouldnt be a problem on travis, but ideally the tests should also be runnable on anyone's local (or remote) machines.
|
|
I can close the socket in the unit test underneath the connection since I'm in the same file. But this only simulates the other end closing. It will verify the logic I'm going to add is sound, but what I don't know is whether the Phobos socket type will register a close when the server closes it, as I don't know how that works. Typical blocking sockets don't tell you they are closed until you try to read from them. I think vibe.d sockets are going to proactively mark the socket closed when that event shows up (which is why I can check it inside autoPurge). It may be more robust to simply check the exception and if the socket is now closed, just ignore the exception during auto-purge. |
|
@Abscissa OK, so I'm ready to make a PR. However, I don't want to use the master branch, because you are working on 2.0.0. I was hoping to just get a 1.2.2 version. Can you make a branch for this? |
terminated the connection. In general resetting the connection now resets all pending states that are moot.
|
In terms of testing, what do I need set up in my environment to to run |
terminated the connection. In general resetting the connection now resets all pending states that are moot.
terminated the connection. In general resetting the connection now resets all pending states that are moot.
|
On January 26, 2018 2:22:27 PM EST, Steven Schveighoffer ***@***.***> wrote:
@Abscissa OK, so I'm ready to make a PR. However, I don't want to use
the master branch, because you are working on 2.0.0. I was hoping to
just get a 1.2.2 version. Can you make a branch for this?
Yea, I was thinking the same about branching. I'll try to take care of that when I get back to my PC tonight.
|
|
On January 26, 2018 2:41:54 PM EST, Steven Schveighoffer ***@***.***> wrote:
In terms of testing, what do I need set up in my environment to to run
`dub test` on the library? I don't want to demolish my existing mysql
server ;)
Nothing special is really needed in your environment. Just access to a mysql server and a special db schema on that server dedicated to mysqln tests ("mysqln-test" should be a good enough name). The tests will restrict themselves to that schema. (but they WILL clobber tables in that schema so again, just make a special one just for mysqln tests.)
Then, run the included './run-tests' script. It will guide you through: The first time you run it, it will create a text file, tell you to put your connection string in the file, and then it will bail to let you do that.
Then just run the script again and it will comple and run all the tests with phobos sockets, then it will repeat using vibe sockets. There are also scripts there to run just the phobos tests or just the vibe ones.
|
|
Ok, I've created branch "v1.2.x", based on the tagged "v1.2.1" |
terminated the connection. In general resetting the connection now resets all pending states that are moot.
|
PR: #155 |
|
Not sure why this didn't auto-close, I thought it would with the commit message. |
…y failing to update parts of it.
autoPurge should check if the socket is open before trying to read from it. I'm having this happen all the time now.
Not sure how to reproduce, as I'm not sure what condition makes the socket close (perhaps even mysql server itself closes it).
The text was updated successfully, but these errors were encountered: