diff options
| author | Roberto Ierusalimschy <roberto@inf.puc-rio.br> | 2019-06-05 13:16:25 -0300 |
|---|---|---|
| committer | Roberto Ierusalimschy <roberto@inf.puc-rio.br> | 2019-06-05 13:16:25 -0300 |
| commit | b4d5dff8ec4f1c8a44db66d368e95d359b04aea7 (patch) | |
| tree | e87fbd3bcbf8a271429ee31f32eaf928058ab376 | |
| parent | 14edd364c3abcb758e74c68a2bdd4ddaeefdae2a (diff) | |
| download | lua-b4d5dff8ec4f1c8a44db66d368e95d359b04aea7.tar.gz lua-b4d5dff8ec4f1c8a44db66d368e95d359b04aea7.tar.bz2 lua-b4d5dff8ec4f1c8a44db66d368e95d359b04aea7.zip | |
Multiple errors in '__toclose' report the first one
When there are multiple errors when closing objects, the error
reported by the protected call is the first one, for two reasons:
First, other errors may be caused by this one;
second, the first error is handled in the original execution context,
and therefore has the full traceback.
| -rw-r--r-- | lcorolib.c | 7 | ||||
| -rw-r--r-- | lfunc.c | 9 | ||||
| -rw-r--r-- | manual/manual.of | 21 | ||||
| -rw-r--r-- | testes/coroutine.lua | 10 | ||||
| -rw-r--r-- | testes/locals.lua | 37 |
5 files changed, 56 insertions, 28 deletions
| @@ -75,11 +75,8 @@ static int luaB_auxwrap (lua_State *L) { | |||
| 75 | int r = auxresume(L, co, lua_gettop(L)); | 75 | int r = auxresume(L, co, lua_gettop(L)); |
| 76 | if (r < 0) { | 76 | if (r < 0) { |
| 77 | int stat = lua_status(co); | 77 | int stat = lua_status(co); |
| 78 | if (stat != LUA_OK && stat != LUA_YIELD) { | 78 | if (stat != LUA_OK && stat != LUA_YIELD) |
| 79 | stat = lua_resetthread(co); /* close variables in case of errors */ | 79 | lua_resetthread(co); /* close variables in case of errors */ |
| 80 | if (stat != LUA_OK) /* error closing variables? */ | ||
| 81 | lua_xmove(co, L, 1); /* get new error object */ | ||
| 82 | } | ||
| 83 | if (lua_type(L, -1) == LUA_TSTRING) { /* error object is a string? */ | 80 | if (lua_type(L, -1) == LUA_TSTRING) { /* error object is a string? */ |
| 84 | luaL_where(L, 1); /* add extra info, if available */ | 81 | luaL_where(L, 1); /* add extra info, if available */ |
| 85 | lua_insert(L, -2); | 82 | lua_insert(L, -2); |
| @@ -144,13 +144,16 @@ static int callclosemth (lua_State *L, TValue *uv, StkId level, int status) { | |||
| 144 | luaG_runerror(L, "attempt to close non-closable variable '%s'", vname); | 144 | luaG_runerror(L, "attempt to close non-closable variable '%s'", vname); |
| 145 | } | 145 | } |
| 146 | } | 146 | } |
| 147 | else { /* there was an error */ | 147 | else { /* must close the object in protected mode */ |
| 148 | ptrdiff_t oldtop = savestack(L, level + 1); | ||
| 148 | /* save error message and set stack top to 'level + 1' */ | 149 | /* save error message and set stack top to 'level + 1' */ |
| 149 | luaD_seterrorobj(L, status, level); | 150 | luaD_seterrorobj(L, status, level); |
| 150 | if (prepclosingmethod(L, uv, s2v(level))) { /* something to call? */ | 151 | if (prepclosingmethod(L, uv, s2v(level))) { /* something to call? */ |
| 151 | int newstatus = luaD_pcall(L, callclose, NULL, savestack(L, level), 0); | 152 | int newstatus = luaD_pcall(L, callclose, NULL, oldtop, 0); |
| 152 | if (newstatus != LUA_OK) /* another error when closing? */ | 153 | if (newstatus != LUA_OK && status == CLOSEPROTECT) /* first error? */ |
| 153 | status = newstatus; /* this will be the new error */ | 154 | status = newstatus; /* this will be the new error */ |
| 155 | else /* leave original error (or nil) on top */ | ||
| 156 | L->top = restorestack(L, oldtop); | ||
| 154 | } | 157 | } |
| 155 | /* else no metamethod; ignore this case and keep original error */ | 158 | /* else no metamethod; ignore this case and keep original error */ |
| 156 | } | 159 | } |
diff --git a/manual/manual.of b/manual/manual.of index 4c9c20b2..2c0957b9 100644 --- a/manual/manual.of +++ b/manual/manual.of | |||
| @@ -1541,11 +1541,17 @@ if there was no error, the second argument is @nil. | |||
| 1541 | 1541 | ||
| 1542 | If several to-be-closed variables go out of scope at the same event, | 1542 | If several to-be-closed variables go out of scope at the same event, |
| 1543 | they are closed in the reverse order that they were declared. | 1543 | they are closed in the reverse order that they were declared. |
| 1544 | |||
| 1544 | If there is any error while running a closing method, | 1545 | If there is any error while running a closing method, |
| 1545 | that error is handled like an error in the regular code | 1546 | that error is handled like an error in the regular code |
| 1546 | where the variable was defined; | 1547 | where the variable was defined; |
| 1547 | in particular, | 1548 | in particular, |
| 1548 | the other pending closing methods will still be called. | 1549 | the other pending closing methods will still be called. |
| 1550 | After an error, | ||
| 1551 | other errors in closing methods | ||
| 1552 | interrupt the respective method, | ||
| 1553 | but are otherwise ignored; | ||
| 1554 | the error reported is the original one. | ||
| 1549 | 1555 | ||
| 1550 | If a coroutine yields inside a block and is never resumed again, | 1556 | If a coroutine yields inside a block and is never resumed again, |
| 1551 | the variables visible at that block will never go out of scope, | 1557 | the variables visible at that block will never go out of scope, |
| @@ -1553,11 +1559,12 @@ and therefore they will never be closed. | |||
| 1553 | Similarly, if a coroutine ends with an error, | 1559 | Similarly, if a coroutine ends with an error, |
| 1554 | it does not unwind its stack, | 1560 | it does not unwind its stack, |
| 1555 | so it does not close any variable. | 1561 | so it does not close any variable. |
| 1556 | You should either use finalizers | 1562 | In both cases, |
| 1557 | or call @Lid{coroutine.close} to close the variables in these cases. | 1563 | you should either use finalizers |
| 1558 | However, note that if the coroutine was created | 1564 | or call @Lid{coroutine.close} to close the variables. |
| 1565 | However, if the coroutine was created | ||
| 1559 | through @Lid{coroutine.wrap}, | 1566 | through @Lid{coroutine.wrap}, |
| 1560 | then its corresponding function will close all variables | 1567 | then its corresponding function will close the coroutine |
| 1561 | in case of errors. | 1568 | in case of errors. |
| 1562 | 1569 | ||
| 1563 | } | 1570 | } |
| @@ -3932,7 +3939,7 @@ Returns a status code: | |||
| 3932 | @Lid{LUA_OK} for no errors in closing methods, | 3939 | @Lid{LUA_OK} for no errors in closing methods, |
| 3933 | or an error status otherwise. | 3940 | or an error status otherwise. |
| 3934 | In case of error, | 3941 | In case of error, |
| 3935 | leave the error object on the stack, | 3942 | leaves the error object on the top of the stack, |
| 3936 | 3943 | ||
| 3937 | } | 3944 | } |
| 3938 | 3945 | ||
| @@ -6355,6 +6362,7 @@ Closes coroutine @id{co}, | |||
| 6355 | that is, | 6362 | that is, |
| 6356 | closes all its pending to-be-closed variables | 6363 | closes all its pending to-be-closed variables |
| 6357 | and puts the coroutine in a dead state. | 6364 | and puts the coroutine in a dead state. |
| 6365 | The given coroutine must be dead or suspended. | ||
| 6358 | In case of error closing some variable, | 6366 | In case of error closing some variable, |
| 6359 | returns @false plus the error object; | 6367 | returns @false plus the error object; |
| 6360 | otherwise returns @true. | 6368 | otherwise returns @true. |
| @@ -6412,7 +6420,8 @@ true when the running coroutine is the main one. | |||
| 6412 | 6420 | ||
| 6413 | Returns the status of the coroutine @id{co}, as a string: | 6421 | Returns the status of the coroutine @id{co}, as a string: |
| 6414 | @T{"running"}, | 6422 | @T{"running"}, |
| 6415 | if the coroutine is running (that is, it called @id{status}); | 6423 | if the coroutine is running |
| 6424 | (that is, it is the one that called @id{status}); | ||
| 6416 | @T{"suspended"}, if the coroutine is suspended in a call to @id{yield}, | 6425 | @T{"suspended"}, if the coroutine is suspended in a call to @id{yield}, |
| 6417 | or if it has not started running yet; | 6426 | or if it has not started running yet; |
| 6418 | @T{"normal"} if the coroutine is active but not running | 6427 | @T{"normal"} if the coroutine is active but not running |
diff --git a/testes/coroutine.lua b/testes/coroutine.lua index 198a5870..f2c0da8b 100644 --- a/testes/coroutine.lua +++ b/testes/coroutine.lua | |||
| @@ -163,15 +163,23 @@ do | |||
| 163 | assert(not X and coroutine.status(co) == "dead") | 163 | assert(not X and coroutine.status(co) == "dead") |
| 164 | 164 | ||
| 165 | -- error closing a coroutine | 165 | -- error closing a coroutine |
| 166 | local x = 0 | ||
| 166 | co = coroutine.create(function() | 167 | co = coroutine.create(function() |
| 168 | local <toclose> y = func2close(function (self,err) | ||
| 169 | if (err ~= 111) then os.exit(false) end -- should not happen | ||
| 170 | x = 200 | ||
| 171 | error(200) | ||
| 172 | end) | ||
| 167 | local <toclose> x = func2close(function (self, err) | 173 | local <toclose> x = func2close(function (self, err) |
| 168 | assert(err == nil); error(111) | 174 | assert(err == nil); error(111) |
| 169 | end) | 175 | end) |
| 170 | coroutine.yield() | 176 | coroutine.yield() |
| 171 | end) | 177 | end) |
| 172 | coroutine.resume(co) | 178 | coroutine.resume(co) |
| 179 | assert(x == 0) | ||
| 173 | local st, msg = coroutine.close(co) | 180 | local st, msg = coroutine.close(co) |
| 174 | assert(not st and coroutine.status(co) == "dead" and msg == 111) | 181 | assert(st == false and coroutine.status(co) == "dead" and msg == 111) |
| 182 | assert(x == 200) | ||
| 175 | 183 | ||
| 176 | end | 184 | end |
| 177 | 185 | ||
diff --git a/testes/locals.lua b/testes/locals.lua index 7834d7da..dccda28f 100644 --- a/testes/locals.lua +++ b/testes/locals.lua | |||
| @@ -267,14 +267,14 @@ do -- errors in __close | |||
| 267 | if err then error(4) end | 267 | if err then error(4) end |
| 268 | end | 268 | end |
| 269 | local stat, msg = pcall(foo, false) | 269 | local stat, msg = pcall(foo, false) |
| 270 | assert(msg == 1) | 270 | assert(msg == 3) |
| 271 | assert(log[1] == 10 and log[2] == 3 and log[3] == 2 and log[4] == 2 | 271 | assert(log[1] == 10 and log[2] == 3 and log[3] == 3 and log[4] == 3 |
| 272 | and #log == 4) | 272 | and #log == 4) |
| 273 | 273 | ||
| 274 | log = {} | 274 | log = {} |
| 275 | local stat, msg = pcall(foo, true) | 275 | local stat, msg = pcall(foo, true) |
| 276 | assert(msg == 1) | 276 | assert(msg == 4) |
| 277 | assert(log[1] == 4 and log[2] == 3 and log[3] == 2 and log[4] == 2 | 277 | assert(log[1] == 4 and log[2] == 4 and log[3] == 4 and log[4] == 4 |
| 278 | and #log == 4) | 278 | and #log == 4) |
| 279 | 279 | ||
| 280 | -- error in toclose in vararg function | 280 | -- error in toclose in vararg function |
| @@ -317,7 +317,7 @@ if rawget(_G, "T") then | |||
| 317 | local <toclose> x = setmetatable({}, {__close = function () | 317 | local <toclose> x = setmetatable({}, {__close = function () |
| 318 | T.alloccount(0); local x = {} -- force a memory error | 318 | T.alloccount(0); local x = {} -- force a memory error |
| 319 | end}) | 319 | end}) |
| 320 | error("a") -- common error inside the function's body | 320 | error(1000) -- common error inside the function's body |
| 321 | end | 321 | end |
| 322 | 322 | ||
| 323 | stack(5) -- ensure a minimal number of CI structures | 323 | stack(5) -- ensure a minimal number of CI structures |
| @@ -325,7 +325,7 @@ if rawget(_G, "T") then | |||
| 325 | -- despite memory error, 'y' will be executed and | 325 | -- despite memory error, 'y' will be executed and |
| 326 | -- memory limit will be lifted | 326 | -- memory limit will be lifted |
| 327 | local _, msg = pcall(foo) | 327 | local _, msg = pcall(foo) |
| 328 | assert(msg == "not enough memory") | 328 | assert(msg == 1000) |
| 329 | 329 | ||
| 330 | local close = func2close(function (self, msg) | 330 | local close = func2close(function (self, msg) |
| 331 | T.alloccount() | 331 | T.alloccount() |
| @@ -368,8 +368,7 @@ if rawget(_G, "T") then | |||
| 368 | end | 368 | end |
| 369 | 369 | ||
| 370 | local _, msg = pcall(test) | 370 | local _, msg = pcall(test) |
| 371 | assert(msg == 1000) | 371 | assert(msg == "not enough memory") -- reported error is the first one |
| 372 | |||
| 373 | 372 | ||
| 374 | do -- testing 'toclose' in C string buffer | 373 | do -- testing 'toclose' in C string buffer |
| 375 | collectgarbage() | 374 | collectgarbage() |
| @@ -453,15 +452,27 @@ end | |||
| 453 | 452 | ||
| 454 | do | 453 | do |
| 455 | -- error in a wrapped coroutine raising errors when closing a variable | 454 | -- error in a wrapped coroutine raising errors when closing a variable |
| 456 | local x = false | 455 | local x = 0 |
| 457 | local co = coroutine.wrap(function () | 456 | local co = coroutine.wrap(function () |
| 458 | local <toclose> xv = func2close(function () error("XXX") end) | 457 | local <toclose> xx = func2close(function () x = x + 1; error("YYY") end) |
| 458 | local <toclose> xv = func2close(function () x = x + 1; error("XXX") end) | ||
| 459 | coroutine.yield(100) | 459 | coroutine.yield(100) |
| 460 | error(200) | 460 | error(200) |
| 461 | end) | 461 | end) |
| 462 | assert(co() == 100) | 462 | assert(co() == 100); assert(x == 0) |
| 463 | local st, msg = pcall(co) | 463 | local st, msg = pcall(co); assert(x == 2) |
| 464 | -- should get last error raised | 464 | assert(not st and msg == 200) -- should get first error raised |
| 465 | |||
| 466 | x = 0 | ||
| 467 | co = coroutine.wrap(function () | ||
| 468 | local <toclose> xx = func2close(function () x = x + 1; error("YYY") end) | ||
| 469 | local <toclose> xv = func2close(function () x = x + 1; error("XXX") end) | ||
| 470 | coroutine.yield(100) | ||
| 471 | return 200 | ||
| 472 | end) | ||
| 473 | assert(co() == 100); assert(x == 0) | ||
| 474 | local st, msg = pcall(co); assert(x == 2) | ||
| 475 | -- should get first error raised | ||
| 465 | assert(not st and string.find(msg, "%w+%.%w+:%d+: XXX")) | 476 | assert(not st and string.find(msg, "%w+%.%w+:%d+: XXX")) |
| 466 | end | 477 | end |
| 467 | 478 | ||
