Question about to-be-closed methods

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Question about to-be-closed methods

Dibyendu Majumdar
Hi,

I am trying to understand this test case in 'locals.lua'

  local x = 0
  local y = 0
  co = coroutine.wrap(function ()
    local xx <close> = func2close(function () y = y + 1; error("YYY") end)
    local xv <close> = func2close(function () x = x + 1; error("XXX") end)
      coroutine.yield(100)
      return 200
  end)
  assert(co() == 100); assert(x == 0)
  local st, msg = pcall(co)
  assert(x == 2 and y == 1)   -- first close is called twice

My question is this:

Why is the 'first close is called twice'?

Thanks and Regards
Dibyendu
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Roberto Ierusalimschy
> I am trying to understand this test case in 'locals.lua'
>
>   local x = 0
>   local y = 0
>   co = coroutine.wrap(function ()
>     local xx <close> = func2close(function () y = y + 1; error("YYY") end)
>     local xv <close> = func2close(function () x = x + 1; error("XXX") end)
>       coroutine.yield(100)
>       return 200
>   end)
>   assert(co() == 100); assert(x == 0)
>   local st, msg = pcall(co)
>   assert(x == 2 and y == 1)   -- first close is called twice
>
> My question is this:
>
> Why is the 'first close is called twice'?

Because it raises an error. When closing a variable normally
(that is, not handling an error), the call to the closing method
may cause a stack overflow, and then the closing method will
not be executed. If that happens, Lua will call the closing method
again after the error (which opened space in the stack).
Lua does not distinguish between stack errors and other errors when
calling the closing method, so any error while running it will call it
again.

A similar argument applies to a memory error. If there is a memory
error while running the closing method, there is a good chance that
the frames freed after the error release enough memory to allow
the closing method to run. So it is worth trying once more.

This is documented:

    If there is any error while running a closing method,
    that error is handled like an error in the regular code
    where the variable was defined.
    However, Lua may call the method one more time.

(If you follow our recommendation that closing methods should be
backed-up by finalizers, they already should be idempotent.)

-- Roberto
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Egor Skriptunoff-2
On Fri, Jul 17, 2020 at 8:43 PM Roberto Ierusalimschy wrote:
    However, Lua may call the method one more time.


Let me summarize.
A to-be-closed variable may be closed 0, 1 or 2 times:
0: due to os.exit()
1: normal behavior
2: due to run-time error in the closing method
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Dibyendu Majumdar
In reply to this post by Roberto Ierusalimschy
On Fri, 17 Jul 2020 at 18:43, Roberto Ierusalimschy
<[hidden email]> wrote:

>
> > I am trying to understand this test case in 'locals.lua'
> >
> >   local x = 0
> >   local y = 0
> >   co = coroutine.wrap(function ()
> >     local xx <close> = func2close(function () y = y + 1; error("YYY") end)
> >     local xv <close> = func2close(function () x = x + 1; error("XXX") end)
> >       coroutine.yield(100)
> >       return 200
> >   end)
> >   assert(co() == 100); assert(x == 0)
> >   local st, msg = pcall(co)
> >   assert(x == 2 and y == 1)   -- first close is called twice
> >
> > My question is this:
> >
> > Why is the 'first close is called twice'?
>
> Because it raises an error. When closing a variable normally
> (that is, not handling an error), the call to the closing method
> may cause a stack overflow, and then the closing method will
> not be executed. If that happens, Lua will call the closing method
> again after the error (which opened space in the stack).

In this case the method was called, one of them is called twice
although both raised errors.

>
> (If you follow our recommendation that closing methods should be
> backed-up by finalizers, they already should be idempotent.)
>

How would you define idempotence for a function that has failed by
raising an error?

Regards
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Roberto Ierusalimschy
> How would you define idempotence for a function that has failed by
> raising an error?

It is Ok to do again whatever it did before the error. (I am sure you
could have guessed it.)

-- Roberto
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Dibyendu Majumdar
On Sat, 18 Jul 2020 at 18:30, Roberto Ierusalimschy
<[hidden email]> wrote:
>
> > How would you define idempotence for a function that has failed by
> > raising an error?
>
> It is Ok to do again whatever it did before the error. (I am sure you
> could have guessed it.)
>

It doesn't really make sense to me, but I am sure there is a good
reason for it. I was trying to figure out where in the code there is a
retry logic - but it isn't obvious. So it is a side effect of the
error handling I assume?

Regards
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Roberto Ierusalimschy
> It doesn't really make sense to me, but I am sure there is a good
> reason for it.

I already explained the reason. If you think it is not a good one, too
bad; it is the only one. Once more: The call can fail due to lack of
resources, but there is a good chance that the error itself will free
those resources (e.g., stack space); so, the next call can succeed where
the previous one failed. The option would be not to repeat the call, and
therefore not to close whatever should be closed.

Note that, if the close method is error-proof, this is entirely
a non issue.


> I was trying to figure out where in the code there is a
> retry logic - but it isn't obvious. So it is a side effect of the
> error handling I assume?

The upvalue is removed from the open list only after the call. If
there is an error, that sequence is interrupted, and the upvalue
is not removed from the list. It would be easy to remove it
before the call, if we preferred not to do the call again.

-- Roberto
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Dibyendu Majumdar
On Tue, 21 Jul 2020 at 17:58, Roberto Ierusalimschy
<[hidden email]> wrote:

>
> > It doesn't really make sense to me, but I am sure there is a good
> > reason for it.
>
> I already explained the reason. If you think it is not a good one, too
> bad; it is the only one. Once more: The call can fail due to lack of
> resources, but there is a good chance that the error itself will free
> those resources (e.g., stack space); so, the next call can succeed where
> the previous one failed. The option would be not to repeat the call, and
> therefore not to close whatever should be closed.

I understood the above. But I do not understand the details, and also
the bigger picture.
When you say 'due to lack of resources' - do you mean a scenario like
failure in malloc()?
If so:

a) In my opinion if malloc() fails all bets should be off - it is very
rare that a program can recover after a malloc failure. In my
experience a program usually cannot recover after a malloc failure in
the real world - because it probably implies failure in multiple
parts, not just Lua. Of course I do not suggest Lua should invoke
abort or exit. Instead Lua immediately return to calling routine with
an error code and the Lua State should be marked 'dead'.

b) What guarantees does Lua try to provide? Is it that every malloc
failure will be followed by GC followed by a retry? If not then why
try in this case?

'It doesn't make sense to me' is my bad way of saying that perhaps the
attempt to recover from certain errors is pointless.

>
> Note that, if the close method is error-proof, this is entirely
> a non issue.
>

Of course. In general it is a bad idea to throw errors from
'destructors' or 'finalizers'.

>
> > I was trying to figure out where in the code there is a
> > retry logic - but it isn't obvious. So it is a side effect of the
> > error handling I assume?
>
> The upvalue is removed from the open list only after the call. If
> there is an error, that sequence is interrupted, and the upvalue
> is not removed from the list. It would be easy to remove it
> before the call, if we preferred not to do the call again.
>

Thank you - I think you mean this commit:
https://github.com/lua/lua/commit/c220b0a5d099372e58e517b9f13eaa7bb0bec45c
?

I did try reverting the commit but it caused some failure in the
tests. But that was just my initial look - I will look deeper.

Regards
Dibyendu
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Roberto Ierusalimschy
> > I already explained the reason. If you think it is not a good one, too
> > bad; it is the only one. Once more: The call can fail due to lack of
> > resources, but there is a good chance that the error itself will free
> > those resources (e.g., stack space); so, the next call can succeed where
> > the previous one failed. The option would be not to repeat the call, and
> > therefore not to close whatever should be closed.
>
> I understood the above. But I do not understand the details, and also
> the bigger picture.
> When you say 'due to lack of resources' - do you mean a scenario like
> failure in malloc()?

As hinted in the message ("stack space"), the paradigmatic scenario
is stack overflow.  When Lua tries to call the close method, or when
the close method tries to call an auxiliar function, there is a stack
overflow, that is, the maximum number of calls allowed by Lua (or the
maximum number of stack slots in use) is exceeded. In this case, the
error will make Lua jumps out of at least one function, opening space
for the close method.

But also we do not have such a grim view of memory allocation failures.
Often the program is doing one particular task that demands lots of
memory. Once that task is aborted, memory is back to normal levels and
the program can continue doing other tasks. (As an analogy, you should
not need to restart your computer if a process is killed by lack of
memory.)

-- Roberto
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Dibyendu Majumdar
On Tue, 21 Jul 2020 at 21:09, Roberto Ierusalimschy
<[hidden email]> wrote:

> As hinted in the message ("stack space"), the paradigmatic scenario
> is stack overflow.  When Lua tries to call the close method, or when
> the close method tries to call an auxiliar function, there is a stack
> overflow, that is, the maximum number of calls allowed by Lua (or the
> maximum number of stack slots in use) is exceeded. In this case, the
> error will make Lua jumps out of at least one function, opening space
> for the close method.

Unfortunately stack overflow is an ambiguous term; because Lua stack
is on the heap, so it can only be heap failure.
The issue with maximum number of calls - I assume that this is a new
concern with Lua 5.4 due to use of C stack for calling Lua functions?
I am curious regarding the reasons for this change in the function
calling approach.

>
> But also we do not have such a grim view of memory allocation failures.
> Often the program is doing one particular task that demands lots of
> memory. Once that task is aborted, memory is back to normal levels and
> the program can continue doing other tasks. (As an analogy, you should
> not need to restart your computer if a process is killed by lack of
> memory.)
>

It is probably an imperfect analogy if you consider Lua as an embedded
language, where it is not the main program. There was a talk several
years ago by a Google engineer that said essentially there is no point
trying to recover after a memory failure. Fail fast is often a better
approach - because trying to recover in that scenario could cause more
damage because of further failures.

Regards
Dibyendu
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Andrew Gierth
>>>>> "Dibyendu" == Dibyendu Majumdar <[hidden email]> writes:

 Dibyendu> It is probably an imperfect analogy if you consider Lua as an
 Dibyendu> embedded language, where it is not the main program. There
 Dibyendu> was a talk several years ago by a Google engineer that said
 Dibyendu> essentially there is no point trying to recover after a
 Dibyendu> memory failure. Fail fast is often a better approach -
 Dibyendu> because trying to recover in that scenario could cause more
 Dibyendu> damage because of further failures.

Most environments are not Google.

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Dibyendu Majumdar
On Tue, 21 Jul 2020 at 22:25, Andrew Gierth <[hidden email]> wrote:

>
> >>>>> "Dibyendu" == Dibyendu Majumdar <[hidden email]> writes:
>
>  Dibyendu> It is probably an imperfect analogy if you consider Lua as an
>  Dibyendu> embedded language, where it is not the main program. There
>  Dibyendu> was a talk several years ago by a Google engineer that said
>  Dibyendu> essentially there is no point trying to recover after a
>  Dibyendu> memory failure. Fail fast is often a better approach -
>  Dibyendu> because trying to recover in that scenario could cause more
>  Dibyendu> damage because of further failures.
>
> Most environments are not Google.
>

I hesitated mentioning Google because I anticipated a response like above :-)

I do not work in the embedded devices space with limited RAM - but in
regular computers, it is pretty hard to get memory failure these days,
because the OS will just start paging.
The most common case of memory failure I have seen are in JVMs where
heap memory is set to a finite limit. So the heap failure is
artificial. It is self imposed rather than imposed by the OS.
I have yet to see a complex program survive after a heap failure.

Regards
Dibyendu
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Viacheslav Usov
In reply to this post by Dibyendu Majumdar
On Tue, Jul 21, 2020 at 10:58 PM Dibyendu Majumdar
<[hidden email]> wrote:

> There was a talk several
> years ago by a Google engineer that said essentially there is no point
> trying to recover after a memory failure. Fail fast is often a better
> approach - because trying to recover in that scenario could cause more
> damage because of further failures.

Either the interpretation is too naive, or the original statement is
nonsensical.

No serious OS will fail catastrophically if it cannot satisfy a memory
request from an application. From an "ordinary" application at least.
The event may be catastrophic for an application, but usually not for
the OS and the other applications.

Lua to its host program is as an application to an OS. Lua does not
allocate memory directly -- it uses an allocator specified by the
host. So it is entirely possible for the host to restrict every
instance of Lua, collectively and individually as it deems fit, in its
use of memory. And it is entirely correct that Lua, as an expressly
embedded language, should deal with memory allocation failures
gracefully.

Cheers,
V.
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Dibyendu Majumdar
On Tue, 21 Jul 2020 at 22:46, Viacheslav Usov <[hidden email]> wrote:

>
> On Tue, Jul 21, 2020 at 10:58 PM Dibyendu Majumdar
> <[hidden email]> wrote:
>
> > There was a talk several
> > years ago by a Google engineer that said essentially there is no point
> > trying to recover after a memory failure. Fail fast is often a better
> > approach - because trying to recover in that scenario could cause more
> > damage because of further failures.
>
> Either the interpretation is too naive, or the original statement is
> nonsensical.
>

For reference
https://youtu.be/NOCElcMcFik
At 38m past
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Viacheslav Usov
On Tue, Jul 21, 2020 at 11:56 PM Dibyendu Majumdar
<[hidden email]> wrote:

> For reference
> https://youtu.be/NOCElcMcFik
> At 38m past

I have listened for a couple of minutes, and at about 38:40 he did
acknowledge "one case where you could credibly recover from that".
Frankly, at least in that particular episode (not handling out of
memory exceptions), his performance  seemed emotional rather than
rational overall.

Cheers,
V.
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Andrew Gierth
In reply to this post by Dibyendu Majumdar
>>>>> "Dibyendu" == Dibyendu Majumdar <[hidden email]> writes:

 Dibyendu> I have yet to see a complex program survive after a heap
 Dibyendu> failure.

PostgreSQL is very good at it, to give just one example.

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Dibyendu Majumdar
On Tue, 21 Jul 2020 at 23:16, Andrew Gierth <[hidden email]> wrote:
>
> >>>>> "Dibyendu" == Dibyendu Majumdar <[hidden email]> writes:
>
>  Dibyendu> I have yet to see a complex program survive after a heap
>  Dibyendu> failure.
>
> PostgreSQL is very good at it, to give just one example.
>

This is becoming OT so maybe we should take it offline. I am curious though.
Database systems tend to pre-allocate essential memory they need. As
long as the DBMS can rollback to a consistent state it doesn't really
care about failure in a stored proc if that is what you are referring
to.

Regards
Dibyendu
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Dibyendu Majumdar
In reply to this post by Dibyendu Majumdar
On Tue, 21 Jul 2020 at 19:55, Dibyendu Majumdar <[hidden email]> wrote:
>
> On Tue, 21 Jul 2020 at 17:58, Roberto Ierusalimschy
> <[hidden email]> wrote:

> > The upvalue is removed from the open list only after the call. If
> > there is an error, that sequence is interrupted, and the upvalue
> > is not removed from the list. It would be easy to remove it
> > before the call, if we preferred not to do the call again.
> >
>
> Thank you - I think you mean this commit:
> https://github.com/lua/lua/commit/c220b0a5d099372e58e517b9f13eaa7bb0bec45c
> ?
>
> I did try reverting the commit but it caused some failure in the
> tests. But that was just my initial look - I will look deeper.
>

If I just move the following lines to where they used to be prior to
above commit:

    if (uv->tbc && status != NOCLOSINGMETH) {
      /* must run closing method, which may change the stack */
      ptrdiff_t levelrel = savestack(L, level);
      status = callclosemth(L, uplevel(uv), status);
      level = restorestack(L, levelrel);
    }

It crashes with memory error when running the tests in local. (I am
currently testing on Windows 10, but will also test on Linux later).
The crash occurs in the first test after printing "testing errors in __close".
So I am unsure what else needs to change...
I would expect test to fail rather than crash.

Regards
Dibyendu
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Andrew Gierth
In reply to this post by Dibyendu Majumdar
>>>>> "Dibyendu" == Dibyendu Majumdar <[hidden email]> writes:

 >> PostgreSQL is very good at it, to give just one example.

 Dibyendu> This is becoming OT so maybe we should take it offline. I am
 Dibyendu> curious though.

Hi, PostgreSQL committer here, please don't try and explain "database
systems" to me.

 Dibyendu> Database systems tend to pre-allocate essential memory they
 Dibyendu> need. As long as the DBMS can rollback to a consistent state
 Dibyendu> it doesn't really care about failure in a stored proc if that
 Dibyendu> is what you are referring to.

That is nothing at all what I am referring to. I'm referring to the
database engine getting a malloc failure at some arbitrary point in the
backend, which could be at any point during query parsing, planning, or
execution, and recovering from it with nothing worse than an "out of
memory" error returned to the client. (In particular, no disconnection
of clients, no processes exiting, no forced recovery of the db.) The
only pre-allocation of memory is a small reserve for error handling
purposes.

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Question about to-be-closed methods

Dibyendu Majumdar
On Wed, 22 Jul 2020 at 00:48, Andrew Gierth <[hidden email]> wrote:
>
> >>>>> "Dibyendu" == Dibyendu Majumdar <[hidden email]> writes:
>
> Hi, PostgreSQL committer here, please don't try and explain "database
> systems" to me.

I knew that hence I asked the question.

>
>  Dibyendu> Database systems tend to pre-allocate essential memory they
>  Dibyendu> need. As long as the DBMS can rollback to a consistent state
>  Dibyendu> it doesn't really care about failure in a stored proc if that
>  Dibyendu> is what you are referring to.
>
> That is nothing at all what I am referring to. I'm referring to the
> database engine getting a malloc failure at some arbitrary point in the
> backend, which could be at any point during query parsing, planning, or
> execution, and recovering from it with nothing worse than an "out of
> memory" error returned to the client. (In particular, no disconnection
> of clients, no processes exiting, no forced recovery of the db.) The
> only pre-allocation of memory is a small reserve for error handling
> purposes.
>

Okay that is good to know.
What about the buffer pool and the buffers used by the write-ahead
log? Aren't they also pre-allocated?
It has been a long time since I looked at PostgreSQL code.

Regards
Dibyendu
12