runtime: recover added in range-over-func loop body doesn't stop panic propagation / segfaults printing error #71675
Comments
gabyhelp
added
the
BugReport
|
Callback behaves normally: package main
import "log"
func main() {
yieldInts(func(x int) bool {
defer func() {
log.Println("recover:called")
recover()
}()
log.Println("x:", x)
return true;
})
}
func yieldInts(yield func(int) bool) {
if !yield(0) {
return
}
log.Println("will panic")
panic("stop")
} |
|
This means that the passing a function to a push iterator section in the containing blog article is not always right. |
Segmentation faults are indeed a significant concern, especially since Golang is a type-safe language and my code does not involve any explicit pointer manipulations. If you are interested, you might want to take another look at the output of this code snippet, as well as a few PlayGround links that demonstrate the issue. |
|
Using callback and using iterator are only equivalent when there are no package main
import "fmt"
func Loop3(yield func() bool) {
if (!yield()) {
return
}
if (!yield()) {
return
}
yield()
}
func main() {
{
defer fmt.Println()
var n = 0
for range Loop3 {
defer fmt.Print(n)
n++
}
}
{
var n = 0
Loop3(func() bool {
defer fmt.Print(n)
n++
return true
})
fmt.Println()
}
} |
seankhliao
changed the title
gopherbot
added
the
compiler/runtime
|
(sorry I only noticed the example posted in the first comments, not the ones on go play) |
If you modify the snippet I provided above to the version below, it will function correctly. However, this alters the semantics of the for loop, and as the official documentation states, traversing an iterator function should be semantically indistinguishable from traversing a slice. Indeed, there are other peculiar phenomena, as demonstrated by the playground links mentioned above. package main
import "log"
func main() {
defer func() {
log.Println("recover:called")
recover()
}()
yieldInts(func(x int) bool {
log.Println("x:", x)
return true
})
}
func yieldInts(yield func(int) bool) {
if !yield(0) {
return
}
log.Println("will panic")
panic("stop")
} |
|
I found another bizarre behavior example: https://go.dev/play/p/gqqk-mrkqpB (changed a bit on Bizarre Behavior two). package main
import "log"
func main() {
for n := range yieldInts {
defer func() {
log.Println(n, "recover:called")
log.Println(recover())
}()
}
panic(3)
}
var i = 0
func yieldInts(yield func(int) bool) {
for {
if i == 2 {
break
}
if !yield(i) {
return
}
i++
}
}It looks the two deferred [edit] sorry, this should be the same as Bizarre Behavior two in principle. |
It is as you have described, but these issues necessitate the skills of more adept individuals to resolve, as they involve the complexities of compilation. Let us await their favorable news. |
|
No need to wait. It is absolutely a bug. The behavior of Bizarre Behavior two the should be the same as package main
import "log"
func main() {
for range 1 {
defer func() {
log.Println("recover:called")
log.Println(recover())
}()
}
panic(3)
} |
Are you the author of the "Go101" series? It's truly a pleasure to meet you here. I've had the chance to read your work, and I must say, it's incredibly detailed and serves as an excellent reference book. It has been of great help to me, and for that, I am deeply grateful. Thank you very much. |
|
Found a similar but different case: #71685 |
|
CC @golang/runtime. |
dmitshur
added
the
NeedsInvestigation
|
The following code shows all kinds of weirdness: package main
import "log"
func main() {
defer func() {
if v := recover(); v != nil {
log.Printf("##, %T, %v\n", v, v)
}
}()
for n := range yieldInts {
log.Println("***", n)
defer func() {
if v := recover(); v != nil {
log.Printf("@@, %T, %v\n", v, v)
}
}()
log.Println("+++", n)
panic(n)
}
}
func yieldInts(yield func(int) bool) {
log.Println("---")
defer func() {
if v := recover(); v != nil {
log.Printf(">>, %T, %v\n", v, v)
panic(v)
}
}()
yield(0)
}Outputs: The loop body is entered twice, which should be once only. The iterator function is also called twice, but should be once only. |
|
It looks a range function is required to resume panicking when it recovered a loop body panic. |
|
A range function is required to not disappear panics from the loop body, that is correct. We felt that it would be confusing for code in a function to have (for example) an obvious panic, that just vanished. I'm still trying to figure out what's going here; it looks like maybe the recover is correctly processed up to the point where it tries to return from the recover, and then things look dubious. I think. But bad control flow across stack frames is going to cause all sorts of chaos. |
Maybe, range functions should not be able to recover panics from loop bodies? It is true that the In the range-over-function way, it is weird to me that to think the code in loop body is called in range functions. |
|
Change https://go.dev/cl/649462 mentions this issue: |
|
Before trying to fix the problem, there is a basic theory problem to be clarified: which of the behavior of the following two programs is correct? (They should behave the same but not now.) package main
import "fmt"
func main() {
defer foo()
}
func foo() {
for range iter {}
panic(123)
}
func iter(yield func(int) bool) {
defer func() {
fmt.Println(recover())
}()
yield(0)
}package main
import "fmt"
func main() {
defer foo()
}
func foo() {
for range iter {
panic(123)
}
}
func iter(yield func(int) bool) {
defer func() {
fmt.Println(recover())
}()
yield(0)
} |
|
Theory 1: The range function is divided into some small functions and the small functions are inlined in the containing function ( Theory 2: The range function is divided into some small functions and the small functions are not inlined in the containing function ( If theory 1 is adopted, then the Whatever, the rule that range functions are required to resume panicking is weird. |
|
Why do you believe that they should behave the same, or that either behavior is incorrect? Also code (in general) should ALWAYS behave the same whether inlining happens or not. In both cases The first We felt that this was the best approach, balancing performance, explainability, testability, real-world utility, and the risk of tinkering with the panic code (the checking code can be expressed as an iterator combinator). This is the 1-variable checking combinator, which may help you understand what is going on: |
|
Panicking in the loop body and after the loop body should have not semantic difference. That is it. Otherwise, it is strange to users. |
Even if an exception is caught, a different exception is thrown again, which might be intended to align with the semantics of the call chain? However, logically, the iterator's recovery could entirely disregard the panic within the loop body. Reprocessing it in this way seems to hold little significance? However, since only the function to which the loop body belongs and its parent functions have the opportunity to recover from a panic triggered by the loop body, this actually contradicts the semantics of the call chain. The official call chain is as follows: the parent function of the loop body -> the iterator function -> the function constructed by the loop body. As a result, the iterator function can only catch a panic but cannot recover it, which seems quite unusual. It appears that there has never been such a case before. Indeed, in the official implementation, there are cases where a parent function cannot catch exceptions from its child functions, while a grandparent function can. However, those parent functions are compiler-generated, such as generating corresponding pointer-type methods for non-pointer type methods. In those scenarios, users cannot access the parent function, allowing for a seamless handling. This situation is different and requires more contemplation? But now that it has been released, it's too late. We'll just have to wait for the official implementation to conform to the current specification constraints. |
The "inline" in my descriptions are NOT the same as the general-speaking "inline". It is in logic and compiler optimization unrelated. [edit]: sorry, missed a "not". |
|
Maybe the best balancing tradeoff approach is to totally disable all If the Theory 1 is adopted, there will be still confusions in the following alike code. package main
import "fmt"
func main() {
defer foo()
panic(123)
}
func foo() {
for range iter {}
}
func iter(yield func(int) bool) {
fmt.Println(recover())
yield(0)
}[edit] Sorry, it looks a new issue is needed to continue the discussion. It seems this is a different problem from both the current issue and #71685. |
|
Change https://go.dev/cl/650476 mentions this issue: |
dmitshur
added
NeedsFix
|
@gopherbot, this needs a backport to 1.24 and 1.23. The fix is simple, and this was supposed to work. |
|
@gopherbot, please backport this. |
|
Backport issue(s) opened: #71839 (for 1.23), #71840 (for 1.24). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases. |
dmitshur
added
the
Critical
Go version
go version go1.23.2 darwin/amd64
Output of
go envin your module/workspace:What did you do?
When traversing an iterative function, there are some peculiar behaviors that arise concerning the defer within the loop, the panic in the outer function surrounding the loop, and the panic within the iterative function itself.
Here is a simple example that can trigger a segmentation fault. Below, I provide several links containing reproductions of a few peculiar behaviors.
output:
What did you see happen?
Bizarre Behavior One
Bizarre Behavior two
Bizarre Behavior three
This issue likely arises from the compiler's handling of the stack when transforming the for loop body into a range function.
What did you expect to see?
I hope to observe behavior that aligns exactly with what is described in this official documentation. It should not include those unpredictable and peculiar behaviors that it does not describe.
The text was updated successfully, but these errors were encountered: