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

Already on GitHub? Sign in to your account

Bug #4786- fix: Mono.Data.Tds.Protocol.TdsConnectionPool.GetConnection() needs to have result set to null before looping back with goto #283

Merged
merged 3 commits into from May 24, 2012

Conversation

Projects
None yet
2 participants
Contributor

robwilkens commented May 1, 2012

See Reported Bug #4786 in bugzilla which this attempts to resolve (and seems to succesfully). This was an error with Open() on System.Data.SqlClient.SqlConnection raising unexpected exception on failure to log in on 2nd attempt and beyond with invalid password/username.

This is a one line change: result = null; had to be set before going back to the 'retry' point in GetConnection..

This is/should-be safe because we had just disconnected result a few lines earlier.

Without this change, under certain conditions which would flag a retry, when it went to the retry point it wouldn't reset 'result', and as a result the main loop at the top of the GetConnection() function would be skipped (it has -- while (result == null) -- at the retry point, and it would always be not null when it reached that on retry), after which point it would just continue to retry on the same result indefinitely, which would eventually result in an exception being raised.

An NUnit test was built and added to this pull request. To build and run just this test, go into mono/mcs/class/Mono.Data.Tds directory and run make test-local to build the test, then run make run-test to run it.

Robert Wilkens added some commits May 1, 2012

Robert Wilkens See Reported Bug #4786 in bugzilla which this attempts to resolve (an…
…d seems to succesfully).

This is a one line change:  result = null; had to be set before going back to the 'retry' point in GetConnection..

This is/should-be safe because we had just disconnected result a few lines earlier.

Without this change, under certain conditions which would flag a retry, when it went to the retry point
it wouldn't reset 'result', and as a result the main loop at the top of the GetConnection() function would
be skipped (it has -- while (result == null) -- at the retry point, and it would always be not null when it reached
that on retry), after which point it would just continue to retry on the same result indefinitely, which would eventually
result in an exception being raised.
58c274a
Robert Wilkens This is the addition of a quick and dirty NUnit test for Bug 4786
This bug was fixed in another commit/push by Robert Wilkens (me).
This NUnit test will fail without the patch (It will raise a
NullReferenceException from within mono), but with the patch for that
bug it will not raise an exception and the test will exit cleanly.

Please note, the top of this patch source sets up a temporary network
socket to listen to a port which pool.GetConnection() simply tries to connect
to.  No data is transfered so nothing needed to be done other than to
call Listen() on that socket which was bound to the port.  If the listen fails,
it means something else is listening already, which is fine, so we ignore
any excpetions from this part of the code (catch and ignore).
fe74cf8
Robert Wilkens This is a minor change to earlier commit which removes a commented out
sql server that should not have been there.  It changes nothing functionally,
but the server is a temporary server, so referencing it didn't make sense.
6df183f
Contributor

robwilkens commented May 3, 2012

Added NUnit test for this particular bug , it looks like it's included in the same earlier pull request...

@migueldeicaza migueldeicaza added a commit that referenced this pull request May 24, 2012

@migueldeicaza migueldeicaza Merge pull request #283 from robwilkens/master
Bug #4786- fix: Mono.Data.Tds.Protocol.TdsConnectionPool.GetConnection() needs to have result set to null before looping back with goto
6dbd060

@migueldeicaza migueldeicaza merged commit 6dbd060 into mono:master May 24, 2012

@xoofx xoofx pushed a commit to xoofx/mono that referenced this pull request Nov 18, 2016

@Tak Tak Merge pull request #283 from Unity-Technologies/debugger-agent-fix-in…
…valid-lmf

Debugger agent fix invalid lmf
9b18cac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment