solver: forward nil return value from sharedOp.Exec #3043
Conversation
solver/jobs.go
Outdated
| @@ -872,6 +872,9 @@ func (s *sharedOp) Exec(ctx context.Context, inputs []Result) (outputs []Result, | |||
| return nil, nil, err | |||
| } | |||
| r := res.(*execRes) | |||
| if r == nil { | |||
There was a problem hiding this comment.
This looks correct but is quite a weird construction. Normally typed nil should be avoided and res=nil should be checked. Maybe a comment saying "callback to s.g.Do guarantees that res is always typed *execRes if err or res is nil" would help.
There was a problem hiding this comment.
Yeah, it's a weird construction. I've reworked the code so that we can just check res==nil.
To do that, I've changed how the method above handles returns - returning s.execRes when s.execRes == nil is not the same as just returning nil, and the new code distinguishes between these two cases.
Exec could sometimes return nil, which would then result in a segmentation fault, as the code attempts to access a field in the nil result to return. This patch introduces a sanity check before accessing any parts of the field. Signed-off-by: Justin Chadwell <me@jedevc.com>
jedevc
force-pushed
the
fix-shared-op-sigsegv
branch
from
f45fb77
to
c58f128
Compare
| return nil, s.execErr | ||
| } | ||
| if s.execRes != nil { | ||
| return s.execRes, nil |
There was a problem hiding this comment.
It looks like this does not atm cache the nil value correctly and Exec gets called again in that case. This can be addressed in follow up.
Is the only case where this can return nil when loading an image without any layers?
Fixes #3040
Exec could sometimes return nil, which would then result in a segmentation fault, as the code attempts to access a field in the nil
result to return. This patch introduces a sanity check before accessing any parts of the field.