Non-deterministic System.AccessViolationException when running tests #43
Comments
|
Copy & paste is the way to go here. I am altering the title a bit, because this exception is thrown regardless of the environment they are being executed in. |
lauxjpn
changed the title
|
Comments copied from the dotnet/runtime PR: When running some of our As it turns our, the underlying cause is the following This is probably related to this issue here. I currently cannot say, whether this is a direct result of a wrong declaration of the This mostly happens after at least 300 calls, but can happen after 700 calls or even never in our test runs. Also note, that this behavior happens on an x64 test run, so there seem to be some issues with those as well and it might not be automatically the case, that the x64 structure definitions (or their marshaling) are correct. Because of your 15K test suite, we are happy to test any fixes. Originally posted by @lauxjpn in dotnet/runtime#33899 (comment) @lauxjpn that sounds like a separate issue to me. You mention that it can happen on both x86 and x64, but do you know if it happens on both .NET Core and .NET Framework? I know that @bubibubi mentioned he encountered issues with large-scale test runs when using System.Data.Ole.Db even on .NET Framework. Originally posted by @FreddyD-GH in dotnet/runtime#33899 (comment) @lauxjpn it could be that we can't run all the test toghether (i.e. we can't include the tests in the CI pipeline). As far as I know it could also be an OleDb Provider (Jet/ACE) issue. Originally posted by @bubibubi in dotnet/runtime#33899 (comment) @FreddyD-GH @bubibubi You guys might be correct. I will open a separate issue for this and will track this bug there down further, because we need to be able to reliably run our tests in CI without non-deterministic exceptions. Originally posted by @lauxjpn in dotnet/runtime#33899 (comment) @lauxjpn , @FreddyD-GH the Jet/ACE provider problem could be related to a specific version. We could also evaluate another solution. There are some tests, about 80 tests, that are integration tests. In my mind this was the minimal set of tests that must run before a release. We could include these tests (and some other MUST run tests) in CI pipeline and run manually other tests. Originally posted by @bubibubi in dotnet/runtime#33899 (comment) @bubibubi Yes, we should narrow down the circumstances of this bug. But it seems you are implying, that the 80 will always run without access violation exception. Is that correct, or do they don't trigger the exception only because they are just so few tests? Originally posted by @lauxjpn in dotnet/runtime#33899 (comment) @lauxjpn I think that the issues on ACE/Jet are related to some cirumnstances:
Also, if I run inside Visual Studio I had a behaviour, if I run with msbuild I had another behaviour. Outside Visual Studio the behaviour is usually better. This is related to GC but adding GC.Collect and GC.WaitForPendingFinalizers does not help. The 80 tests just works always. It does not matter the order they run. Originally posted by @bubibubi in dotnet/runtime#33899 (comment) |
|
I'm wondering if Marshal.AreComObjectsAvailableForCleanup would be helpful in this situation. I've never had to use this method before, but I think the documentation suggests that you'd use it like this: while(Marshal.AreComObjectsAvailableForCleanup())
{
GC.Collect();
GC.WaitForPendingFinalizers();
}I believe @bubibubi said he is already using Even if it works, this seems like it's still not ideal, but might be good enough for now. |
|
I will dig deeper into this issue, now that the general upgrade is done. First, I am going to build a scenario that is simpler and more flexible to debug than the xUnit test suit, and that is able to trigger the exception in a more reliable manner. Then I will try the Finally, I will take a look at the processor instructions to find out what the ACE database driver expects and what it actually gets. |
|
I created the smallest possible sample to reproduce the
A couple of examples that will throw: SELECT [c].[Address]
FROM [Customers] AS [c]
WHERE [c].[City] = 'Berlin'
UNION
SELECT [c0].[Address]
FROM [Customers] AS [c0]
WHERE [c0].[City] = 'London';
SELECT 1
FROM [Customers] AS [c];SELECT [c].[Address], [c].[Address]
FROM [Customers] AS [c]
WHERE [c].[City] = 'Berlin'
UNION
SELECT [c0].[Address], [c0].[Address]
FROM [Customers] AS [c0]
WHERE [c0].[City] = 'London';
SELECT 1, NULL
FROM [Customers] AS [c];SELECT [c].[Address], [c].[Address]
FROM [Customers] AS [c]
WHERE [c].[City] = 'Berlin'
UNION
SELECT [c0].[Address], [c0].[Address]
FROM [Customers] AS [c0]
WHERE [c0].[City] = 'London';
SELECT 1, NULL
FROM [Customers] AS [c];A couple of examples that will not throw: SELECT [c].[Address]
FROM [Customers] AS [c]
WHERE [c].[City] = 'Berlin'
UNION
SELECT [c0].[Address]
FROM [Customers] AS [c0]
WHERE [c0].[City] = 'London';
SELECT [c].[CustomerID] /* <-- not a scalar value */
FROM [Customers] AS [c];SELECT [c].[Address]
FROM [Customers] AS [c]
WHERE [c].[City] = 'Berlin'; /* <-- not a UNION */
SELECT 1
FROM [Customers] AS [c];SELECT [c].[Address], [c].[Address]
FROM [Customers] AS [c]
WHERE [c].[City] = 'Berlin'
UNION
SELECT [c0].[Address], [c0].[Address]
FROM [Customers] AS [c0]
WHERE [c0].[City] = 'London';
SELECT 1 /* <-- fewer scalar values than fields in UNION */
FROM [Customers] AS [c];SELECT [c].[Address], [c].[Address]
FROM [Customers] AS [c]
WHERE [c].[City] = 'Berlin'
UNION
SELECT [c0].[Address], [c0].[Address]
FROM [Customers] AS [c0]
WHERE [c0].[City] = 'London';
SELECT 1, [c].[CustomerID] /* <-- not just scalar values */
FROM [Customers] AS [c];SELECT [c].[Address]
FROM [Customers] AS [c]
WHERE [c].[City] = 'Berlin'
UNION
SELECT [c0].[Address]
FROM [Customers] AS [c0]
WHERE [c0].[City] = 'London';
SELECT *
FROM [Orders] AS [o]; /* <-- table differs from the one used in the UNION */The following code can be used to reproduce the issue. The code should be run with the latest nightly-build of using System;
using System.Data.OleDb;
namespace JetProviderExceptionTests
{
internal static class Program
{
private static void Main()
{
Console.WriteLine($"Running as {(Environment.Is64BitProcess ? "x64" : "x86")} process.");
var tagDBPARAMBINDINFOName = "tagDBPARAMBINDINFO" + (Environment.Is64BitProcess
? string.Empty
: "_x86");
// Is 2 on x86 and 8 on x64.
Console.WriteLine($"{tagDBPARAMBINDINFOName} field alignment is {Type.GetType($"System.Data.OleDb.{tagDBPARAMBINDINFOName}, System.Data.OleDb").StructLayoutAttribute.Pack}.");
// Is 8 on both x86 and x64.
Console.WriteLine($"tagDBPARAMS field alignment is {Type.GetType("System.Data.OleDb.tagDBPARAMS, System.Data.OleDb").StructLayoutAttribute.Pack}.");
Console.WriteLine();
Console.WriteLine("Press any key to start...");
Console.ReadKey();
RunNorthwindTest();
}
private static void RunNorthwindTest()
{
try
{
using var connection = new OleDbConnection("Provider=Microsoft.ACE.OLEDB.16.0;Data Source=Northwind.accdb");
connection.Open();
for (var i = 0; i < 1000; i++)
{
Console.WriteLine($"{i:000}");
//
// Select_Union:
//
using (var command1 = connection.CreateCommand())
{
command1.CommandText = @"SELECT [c].[Address]
FROM [Customers] AS [c]
WHERE [c].[City] = 'Berlin'
UNION
SELECT [c0].[Address]
FROM [Customers] AS [c0]
WHERE [c0].[City] = 'London'";
using (var dataReader1 = command1.ExecuteReader())
{
while (dataReader1.Read())
{
}
}
}
/*
using (var command15 = connection.CreateCommand())
{
command15.CommandText = @"SELECT [c].[Address]
FROM [Customers] AS [c]
WHERE [c].[City] = 'Madrid'";
using (var dataReader15 = command15.ExecuteReader())
{
while (dataReader15.Read())
{
}
}
}
*/
//
// Select_bool_closure:
//
using (var command2 = connection.CreateCommand())
{
command2.CommandText = @"SELECT 1
FROM [Customers] AS [c]";
using (var dataReader2 = command2.ExecuteReader())
{
while (dataReader2.Read())
{
}
}
}
}
}
catch (AccessViolationException e)
{
Console.WriteLine(e);
}
}
}
}The code uses the version of the Northwind database we are also now using in our functional tests: There are no transactions and no multi-threading involved to trigger the exception. /cc @saurabh500 |
|
@lauxjpn Is this on .NET Core only? Or does it happen with .NET Framework as well? |
|
@FreddyD-GH The exception is being thrown in a .Net Framework 4.7.2 project using the .NET Framwork version of OLE DB and running in a 64 bit process as well: The same test works fine under .Net Framework 4.7.2 (x64) when using ODBC and the @saurabh500 With Access nowadays preferring DAO again over ADO, is OLE DB still the right choice for maximum compatibility or should we switch to ODBC? We could also support both and let the user decide (with a reasonable default; probably ODBC as it looks right now). |
|
@lauxjpn FYI, I tried the code you provided and was able to reproduce the behavior. I've seen a good number of cases like this where having resources that aren't cleaned up properly is the culprit, but that doesn't appear to apply in this situation (at least not on the managed side). I didn't get a chance to really dig into it, though. As far as ODBC, I ended up trying that on a .NET Core project I had when I initially had the issues with OleDb x86. I can't remember the specific issues, but I remember not getting any further with ODBC than I did with OleDb (and OleDb x86 on .NET Core is broken AF). I think I remember ODBC was not as buggy as OleDb, but instead I think it gave me much nicer errors that it didn't support certain things. I didn't spend much time trying to find workarounds, though. Regardless, ODBC may be worth looking into, but my memory says not to get your hopes up. |
|
@FreddyD-GH I will test a parallel implementation, where people can choose which of the two technologies to choose as their base. Most of the code has already been abstracted away by @bubibubi, so the implementation cost should be relatively low. |
|
As far as I remember we can't avoid to use OleDb because we risk to loose code from DB features. |
I will take a closer look at that. Generally, the
|
|
@lauxjpn I updated the comment sorry. I think that with ODBC we can't retrieve the tables list. |
|
@bubibubi No Problem. I will take a closer look at this, but I am very confident, that we will be able to get the table list from ODBC. If this should turn out to be not possible for some reason, we can always get it directly from the
ADOX is part of the Windows DAC, a modern version of the traditional MDAC, that ships as part of windows. You can find the x86 version of ADOX in |
|
@lauxjpn there is a small issue about |
We can use the OdbcConnection.GetSchema Method. It can be used like in the following line, to get the table schemas: myOdbcConnection.GetSchema("tables");It can also be used in conjunction with restrictions:
Further information: |
|
I think we should do a proof of concept. About retrieving schema, I can't remember the issues (it's a work I've done around 2013 for the first EF 6 provider) but I remember that I evaluated OleDb, ODBC and a full stack provider. |
|
About AccessViolationException did you try to run tests disabling connection pooling? I've implemented a connection pooling inside System.Data.Jet because I had some issues with OleDb connection pooling. We could check if now OleDb connection pooling works fine and remove System.Data.Jet connection pooling implementation |
Yes, its one of the first things I checked, because of my previous post on StackOverflow. It made no difference. I just extended the BTW, we will need to revisit the connection pool implementation in |
I mean the System.Data.Jet implementation of connection pooling. It's not implemented in JetEntityFrameworkProvider |
|
About Jet/ACE provider for OleDb, there is an issue related to the wrong data type determined in union queries when you use const values, for example If I think that it could be related to this issue (also if in your latest tests there are not any const). This is a sample of a partial workaround applied to JetDataReader |
Yes, the first
Well, something in the SELECT [u].[Address]
FROM (
SELECT [c].[Address]
FROM [Customers] AS [c]
WHERE [c].[City] = 'Berlin'
UNION
SELECT [c0].[Address]
FROM [Customers] AS [c0]
WHERE [c0].[City] = 'London'
) as [u]We should do this anyway in our SQL generation, because Jet/ACE's parsing seems to have problems with more advanced scenarios together with
The sample code I provided does not use any of our Jet libraries at all, but works directly with |
|
Are we fine with referencing the preferred data access api package (e.g. It would mean, that the user would need to do nothing additionally to run Jet in the default configuration. It would also mean, that if he wants to use another data access api (e.g. The other way would be for The benefits to this are, that users will not end up with unused data access api references and it forces a user to make a conscious decision which api to use. But he might choose one that is more buggy then (like OLE DB). I currently implemented the second approach (so no need to reference any data api at all for us at the moment). But we can easily switch to the first approach by just making one api the preset default value and adding the reference to the |
My opinions: --
I think that also Code from DB is a must, I've done it for other databases. I think that usually Is used one time per project but when you start working on an existing project it's a great help. -- |
If we use a multi-package approach, we would have 3 packages (which is fine):
But we should be able to archive the same with just our main package and making it mandatory for the user to choose a data access api (this is how I have implemented it at the moment). I will push a PR with this approach shortly (today or tomorrow), so that everybody can take it out for a spin and build his own opinion about it. @bubibubi Thinking about this a bit more, we could actually do both. The base package will run just fine on its own, but the additional data access packages (that will be very small) would just add additional
Are you talking about Jet/ACE's inability to convert decimal values or its sorting problems with decimal values?
I will test the But generally, ACE/Jet has always been quirky, buggy and limited in its scope. I never did really fancy queries in the past, because I am aware of its limitations. I always do simple enough queries against the data store and do the fancy stuff client-side with LINQ-to-Objects.
I agree. Scaffolding and Migrations are mandatory features. |
|
Interestingly, we could actually just deduce the data access api from the connection string style. |
|
As for the I will also do a test with my dusty C++ compiler directly against the COM interfaces. With this test we can distinguish between an error between |
Starting a separate issue for conversation that began with:
dotnet/runtime#33899 (comment)
@lauxjpn @bubibubi Does anyone know if GitHub would allow us to move the comments from that PR into this issue instead, or do we have to manually copy/paste each one?
The text was updated successfully, but these errors were encountered: