search

cue-lang / cue

The home of the CUE language! Validate and define text-based and dynamic configuration
https://cuelang.org
Apache License 2.0
5.15k stars 297 forks source link

Detect and warn on infinite structural cycles #2398

Open disintegrator opened 1 year ago

disintegrator commented 1 year ago

Is your feature request related to a problem? Please describe.

I was testing CUE 0.5.0 to see if it addressed #1827 and found that cue export and cue vet were hanging indefinitely. I wrongly attributed this to a regression in CUE. After more digging into a small reproduction I created, I found that the issue was in how I was generating CUE code in Benthos (using benthos list --format cue). It turns out that one of the definitions we generate contained an infinite cycle. I've submitted a fix for this to Benthos and confirmed that CUE 0.5.0 was working fine. The entire context behind the issue can be found in #1827.

Describe the solution you'd like

It would've been nice if CUE had flagged this type of recursive definition (demonstrated here). It wasn't intended and it resulted in the cue commands hanging. Flagging would've helped identify the underlying issue in Benthos sooner.

Describe alternatives you've considered

No alternatives considered.

Additional context

The context behind this feature request can be found in #1827.

mvdan commented 1 year ago

I think this is more of a bug than a feature request - CUE shouldn't get stuck or burn lots of CPU if there's a cycle :) For example, look at https://github.com/cue-lang/cue/issues/2367, although that one was worse as it resulted in a "stack limit exceeded" panic.

mvdan commented 1 year ago

Also, have you checked if this also happens with older versions of CUE? If not, it would be interesting to bisect with your small reproducer.

disintegrator commented 1 year ago

@mvdan I'm almost certain nothing changed about the generated Benthos schema besides inclusion of comments and it was all working with CUE v0.4.3. I confirmed this again just now. It still stands that the Benthos definition for #Processor was incorrect but at least CUE v0.4.3 wasn't hanging.

myitcv commented 1 year ago

Marking tentatively as v0.6.0, because per @mvdan this shouldn't be happening. With the changes in cycle detection, we should look closely at what's going on here.

mpvl commented 1 year ago

For completeness, the cyclic code referred to is here:

#Processor: {log:{message:string}} | {cache:{operation:string}}
#Processor: #Processor & {
    label?: string
    processors?: [...#Processor] // warn me about this
}

There are two cycles here: the #Processor & { is a no-op. (like a == a) So here a cue vet could suggest to eliminate it. It should not affect evaluation, however.

The second one is fine as well. Using optional fields to break cycles is a valid approach. So I'm not sure how CUE would see that this is an undesirable cycle.

alex1yaremchuk commented 1 week ago

Met this issue while playing with Bento (fork of Benthos) schema. It hangs with Bento generated schema and works with it being simplified.

Part with Processor seems to already have mentioned bug fixed. but probably config has some other cycle and CUE cannot tell about it.

`#Processor: or([for name, config in #AllProcessors { (name): config }])

Processor: {

label?: string

} `