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

Lots of exceptions by transaction destructor [DNET657] #614

Closed
firebird-automations opened this issue Dec 22, 2015 · 15 comments
Closed

Lots of exceptions by transaction destructor [DNET657] #614

firebird-automations opened this issue Dec 22, 2015 · 15 comments

Comments

@firebird-automations
Copy link

Submitted by: Alexander Muylaert-Gelein (gonline)

Attachments:
ConsoleApplication1.7z
ConsoleApplication1 4.8.0.7z
4.8.png
4.9.png

FbTransaction doesn't dispose the internal GDSTransaction anymore.
If you look at FbTransaction.Commit, you'll notice it calls UpdateTransaction. This method just sets the _transaction to null. The dispose of this internal transaction isn't being called.

The disadvantage of this is that I have a lot of unwanted errors being spawned inside the gc thread. If you replace the code in fbtransaction by the following, it seems to work fine

	private void UpdateTransaction\(\)
	\{
		if \(\_connection \!= null && \_connection\.InnerConnection \!= null\)
		\{
			\_connection\.InnerConnection\.TransactionUpdated\(\);
		\}

		\_isUpdated = true;
		\_connection = null;
        if \(\_transaction \!= null\)
        \{
            \_transaction\.Dispose\(\);
            \_transaction = null;
        \}
    \}
@firebird-automations
Copy link
Author

Modified by: @cincuranet

status: Open [ 1 ] => In Progress [ 3 ]

@firebird-automations
Copy link
Author

Commented by: @cincuranet

Provide a code that allows to test this.

@firebird-automations
Copy link
Author

Commented by: Alexander Muylaert-Gelein (gonline)

attached a project. you can refer it to any db off course

@firebird-automations
Copy link
Author

Modified by: Alexander Muylaert-Gelein (gonline)

Attachment: ConsoleApplication1.7z [ 12858 ]

@firebird-automations
Copy link
Author

Commented by: Alexander Muylaert-Gelein (gonline)

Hi Jiri

I just tested this console with the 4.8.0 and... it doesn't have the exceptions.

So 4.9 has it, 4.8 hasn't it

@firebird-automations
Copy link
Author

Modified by: Alexander Muylaert-Gelein (gonline)

Attachment: ConsoleApplication1 4.8.0.7z [ 12859 ]

@firebird-automations
Copy link
Author

Commented by: @cincuranet

Sure the 4.8 has it. Your test is just wrong and does not show it.

@firebird-automations
Copy link
Author

Commented by: @cincuranet

I don't think the
if (_transaction != null)
{
_transaction.Dispose();
_transaction = null;
}
will solve it. It will call the `Dispose` anyway and the `CheckTransactionState` will throw.

@firebird-automations
Copy link
Author

Commented by: Alexander Muylaert-Gelein (gonline)

Hi Jiri

In visual studio you can clearly see how 4.8 and 4.9 behave different. The 4.8 throws no error in the test app, if I upgrade firebird it goes kaboem a lot.

@firebird-automations
Copy link
Author

Modified by: Alexander Muylaert-Gelein (gonline)

Attachment: 4.8.png [ 12862 ]

Attachment: 4.9.png [ 12863 ]

@firebird-automations
Copy link
Author

Commented by: @cincuranet

Sure. Because your test is not 100%. But the error is still there (in provider as well as in your test).

@firebird-automations
Copy link
Author

Modified by: @cincuranet

status: In Progress [ 3 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

Fix Version: vNext [ 10742 ]

@firebird-automations
Copy link
Author

Commented by: Alexander Muylaert-Gelein (gonline)

Could it be that this change happened between 4.8.0 a,d 4.8.1. Today I had to revert to 4.8.X and with the 8.1 I still had the same issue. with 4.8.0 I didn't had it at all.

@firebird-automations
Copy link
Author

Commented by: Alexander Muylaert-Gelein (gonline)

Hi Jiri

The reason why you're having this since 4.8.1 is because you solved a bug. This will teach you solving bugs in existing source code!

In 4.8 you had the GC.SuppressFinalize(this); in the constructor of ExtTransaction.

By removing this line from the constructor (what is the right thing to do), you off course trigger the actual issue of the exceptions during the destructor. Although the try catch donothing end, makes it hidden, I would really ask you to solve it gracefully.

@firebird-automations
Copy link
Author

Commented by: @cincuranet

ExtTransaction class is never used. You probably mean GdsTransaction. Anyway, the bug was there even before, it was just not surfaced by standard calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants