Deleting Lock used with AST routine

[Adriano dos Santos Fernandes]

Hi!

I've created a test showing a problem with Lock deletion when it's used
with an AST routine.

We have this pattern in code, for example in Routine::remove:

// deallocate all structure which were allocated. The routine
// blr is originally read into a new pool from which all request
// are allocated. That will not be freed up.

if (existenceLock)
{
delete existenceLock;
existenceLock = NULL;
}

Here is test case:

https://github.com/asfernandes/firebird/commit/8228caa27f1b4271a2da57345c439964f807f50a

This test always pass, but if we comment line "1!" and uncomment line
"2!" it crashes many times calling LCK_release inside blockingAst, as
the object is already deleted.

Do we have any safe approach to do it or the only way would be to let
objects die in the attachment/database pools after LCK_fini?


Adriano

[Dmitry Yemanov]

I don't think we have any safe solution. But I'm wondering why we would
ever want to delete a lock under concurrent load? It may be either
locked or not, but why should it be non-existent?


Dmitry

[Adriano dos Santos Fernandes]

Em dom., 20 de nov. de 2022 08:01, Dmitry Yemanov <fire...@yandex.ru> escreveu:

I don't think we have any safe solution. But I'm wondering why we would
ever want to delete a lock under concurrent load? It may be either
locked or not, but why should it be non-existent?

But defer delete until attachment release when there are no reason for it is neither good.

So I think we should hide Lock constructor, wrap it in Lock::create that returns a ref counted object and have users and internals manage it correctly.

Adriano

[Dimitry Sibiryakov]

Adriano dos Santos Fernandes wrote 20.11.2022 13:17:
> But defer delete until attachment release when there are no reason for it is
> neither good.

Is it so big and created in big amount?

--
WBR, SD.

[Vlad Khorsun]

20.11.2022 14:17, Adriano dos Santos Fernandes wrote:

> Em dom., 20 de nov. de 2022 08:01, Dmitry Yemanov <fire...@yandex.ru <mailto:fire...@yandex.ru>> escreveu:
>
>
> I don't think we have any safe solution. But I'm wondering why we would
> ever want to delete a lock under concurrent load? It may be either
> locked or not, but why should it be non-existent?
>
>
> But defer delete until attachment release when there are no reason for it is neither good.
>
> So I think we should hide Lock constructor, wrap it in Lock::create that returns a ref counted object and have users and internals
> manage it correctly.

Don't hurry with conclusions. With all "problems" code works ok for many years.
I'll look at your test closer and report my findings.

Regards,
Vlad