attic-labs / noms

The versioned, forkable, syncable database
Apache License 2.0
7.45k stars 267 forks source link

Crash importing blob #3837

Closed aboodman closed 5 years ago

aboodman commented 5 years ago

I have a blob that crashes like so:

!? noms blob put myblob /tmp/db
        Error Trace:    try.go:99
                    try.go:34
                    sequence_concat.go:24
                    blob.go:136
                    blob.go:240
                    blob.go:212
                    noms_blob_put.go:55
                    noms_blob.go:32
                    noms.go:100
                    proc.go:201
                    asm_amd64.s:1333
    Error:          cannot concat sequences from different databases

goroutine 1 [running]:
github.com/attic-labs/noms/go/d.Panic(0x185888a, 0x30, 0x0, 0x0, 0x0)
    /Users/aa/src/github.com/attic-labs/noms/go/d/try.go:34 +0x10a
github.com/attic-labs/noms/go/types.concat(0x1b26700, 0xc0000b59f0, 0x1b26700, 0xc0000b4190, 0xc000fada78, 0x1b26700, 0xc0000b59f0)
    /Users/aa/src/github.com/attic-labs/noms/go/types/sequence_concat.go:24 +0xe6
github.com/attic-labs/noms/go/types.Blob.Concat(0x1b26700, 0xc0000b59f0, 0x1b26700, 0xc0000b4190, 0x1b26700, 0xc0000b59f0)
    /Users/aa/src/github.com/attic-labs/noms/go/types/blob.go:136 +0x7e
github.com/attic-labs/noms/go/types.readBlobsP(0x1b1e260, 0xc000214ae0, 0xc0000b33e0, 0x6, 0x6, 0x0, 0x1b1e260)
    /Users/aa/src/github.com/attic-labs/noms/go/types/blob.go:240 +0x1c8
github.com/attic-labs/noms/go/types.NewBlob(0x1b1e260, 0xc000214ae0, 0xc0000b33e0, 0x6, 0x6, 0x0, 0x0)
    /Users/aa/src/github.com/attic-labs/noms/go/types/blob.go:212 +0x53
main.nomsBlobPut(0x7ffeefbffbe6, 0x3a, 0x7ffeefbffc21, 0x19, 0x8, 0x0)
    /Users/aa/src/github.com/attic-labs/noms/cmd/noms/noms_blob_put.go:55 +0x492
main.nomsBlob.func1(0xc0000a9a80, 0x8, 0xc0000a9a80)
    /Users/aa/src/github.com/attic-labs/noms/cmd/noms/noms_blob.go:32 +0x196
main.main()
    /Users/aa/src/github.com/attic-labs/noms/cmd/noms/noms.go:100 +0xd78

The crash is here:

https://github.com/attic-labs/noms/blob/master/go/types/sequence_concat.go#L24

What happens is that vrw is *ValueStore and snd is *database, where snd.ValueStore == vrw. IOW, the database's ValueStore gets unwrapped somewhere along the line and used for vrw.

This kind of accidental unwrapping is common, and usually happens by calling a method on the embedded field. That's what happens to us too:

That call is where the ValueStore gets unwrapped and eventually passed through to the new value.

This all said, I don't think there's anything actually wrong here other than the bad assertion. It's not valid to assume that if two ValueReadWriter values are non-equal that they wrap different database instances.

Also, I don't think this assertion is particularly needed, since it is just a special case of doing anything (except diff and sync) with values from different databases.