Check nil worker ref in llbBridgeForwarder.

[sipsma]

Without this, the included test case will result in buildkitd panicking.

Signed-off-by: Erik Sipsma erik@sipsma.dev

Diagnosed here: dagger/dagger#3330 (comment)

Also possibly related to #3134 (unless that one was already fixed)

[tonistiigi]

It seems logical for the getImmutableRef to return nil in this case, even if that makes the patch longer. Otherwise maybe the name should be must*

[tonistiigi]

Shouldn't the ref also be nil in this case. Or at least there should be some saner way to check for nil ref from the client side.

[sipsma]

I agree but it seems tricky to deal with the case where evaluate isn't true. You won't know the actual result is nil until its evaluated, but you have to return something to represent it until then.

I guess it would be possible to only return nil only when evaluate is true, but then you still have to deal with the case where something is "lazily nil" when evaluate is false, so I don't know if that actually simplifies anything overall.

[sipsma]

It seems logical for the getImmutableRef to return nil in this case, even if that makes the patch longer. Otherwise maybe the name should be must*

Yeah makes sense. I realized that all the methods in llbBridgeForwarder use cacheutil, which actually handles nil mountables perfectly already. So I updated to just passthrough to that without trying to use the nil and it all seems to work. ReadDir, ReadFile, etc. all have logical behavior (basically just treat a nil mountable as an empty mount). Think this makes a lot of sense, let me know if you agree.