Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dean's LMDB on windows crash #950

Closed
warner opened this issue Apr 16, 2020 · 10 comments · Fixed by #985
Closed

dean's LMDB on windows crash #950

warner opened this issue Apr 16, 2020 · 10 comments · Fixed by #985
Assignees
Labels
SwingSet package: SwingSet

Comments

@warner
Copy link
Member

warner commented Apr 16, 2020

@dtribble experienced a crash as soon as swingset tried to read a key from the database

@warner
Copy link
Member Author

warner commented Apr 16, 2020

https://bugs.openldap.org/buglist.cgi?product=LMDB&component=liblmdb&resolution=--- is the LMDB bug tracker in case it helps.

@warner warner added the SwingSet package: SwingSet label Apr 16, 2020
@dtribble
Copy link
Member

The crash is in running tests for swing-store-lmdb. With some debugging, we determined that it failed on the first read after writing the initial DB.

tribble@Alien:~/git/916-agoric/packages/swing-store-lmdb$ yarn test
yarn run v1.22.4
$ tape -r esm 'test/**/test*.js' | tap-spec

  storageInLMDB

Error: MDB_BAD_TXN: Transaction must abort, has a child, or is invalid
    at get (/mnt/c/dev/git/916-agoric/packages/swing-store-lmdb/lmdbSwingStore.js:63:22)
    at Object.has (/mnt/c/dev/git/916-agoric/packages/swing-store-lmdb/lmdbSwingStore.js:115:12)
    at testStorage (/mnt/c/dev/git/916-agoric/packages/swing-store-lmdb/test/test-state.js:9:19)
    at Test.<anonymous> (/mnt/c/dev/git/916-agoric/packages/swing-store-lmdb/test/test-state.js:41:3)
    at Test.bound [as _cb] (/mnt/c/dev/git/916-agoric/node_modules/tape/lib/test.js:77:32)
    at Test.run (/mnt/c/dev/git/916-agoric/node_modules/tape/lib/test.js:93:10)
    at Test.bound [as run] (/mnt/c/dev/git/916-agoric/node_modules/tape/lib/test.js:77:32)
    at Immediate.next (/mnt/c/dev/git/916-agoric/node_modules/tape/lib/results.js:81:19)
    at processImmediate (internal/timers.js:456:21)
    at process.topLevelDomainCallback (domain.js:137:15) {
  code: -30782
}


error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.```

@dtribble
Copy link
Member

The failure occurs in WSL (Windows Subsystem for Linux), on master branch.

@warner
Copy link
Member Author

warner commented Apr 16, 2020

https://wiki.ubuntu.com/WSL has some notes on WSL

@warner
Copy link
Member Author

warner commented Apr 16, 2020

I found one bug filed against WSL that mentioned lmdb, maybe unrelated: microsoft/WSL#3451

@FUDCo
Copy link
Contributor

FUDCo commented Apr 16, 2020

I think that bug is unrelated, since we're not dealing with a zero length file here, but good to keep a pointer to it just in case.

@warner
Copy link
Member Author

warner commented Apr 16, 2020

Appveyor is a windows CI platform Ilm using on other projects, and appveyor/ci#1295 describes something about maybe supporting WSL therein.

@warner
Copy link
Member Author

warner commented Apr 16, 2020

CircleCI mentioned adding windows support: https://circleci.com/docs/2.0/hello-world-windows/

warner added a commit that referenced this issue Apr 18, 2020
This is a temporary workaround for #950, and should be reverted (i.e. use
LMDB unconditionally) when the LMDB crash on "Windows Subsystem for Linux" is
fixed.
warner added a commit that referenced this issue Apr 18, 2020
fix: cosmic-swingset: fall back to SimpleStore if LMDB doesn't work

refs #950
@warner
Copy link
Member Author

warner commented Apr 21, 2020

@warner warner mentioned this issue Apr 21, 2020
@FUDCo
Copy link
Contributor

FUDCo commented Apr 23, 2020

So I think we have finally tracked down the problem and killed it. Heres the story:

LMDB has two different ways it can put stuff into its database file: writing to memory addresses that are mapped onto the file and by explicit I/O operations that write the file directly. In its normal operation, LMDB uses both of these techniques. Apparently, something about the WSL environment (perhaps the way it manages the Linux file system, which, it seems, is not ext3 or ext4) means that its implementation of mmap doesn't work quite the same way as it does elsewhere (arguably this might even be a bug in the filesystem implementation, but it may have been an explicit design tradeoff; I just don't know). If you map a file into memory, most systems give you a block of address space as large as the allocated length of the file (a length set using ftruncate which, despite its name, can also make a file larger). However, the actual file as you see it on disk is only as big as the highest position in it that has data. This is why you generally set the LMDB file size to something very large like 2GB even though the working data.mdb file could be as small as 8K or 12K as it is in the test that was failing. LMDB "grows" the file by writing onto its end using write or pwrite.

However, in the file system that Dean's machine is using, memory corresponding to space beyond the length that the file was at the time mmap was initially called doesn't actually get filled with stuff written to the file via write calls. Instead, you see a bunch of zeros. Linux and BSD have a system call named msync to sync memory with disk, but what it really does is write the memory to disk, i.e., the state in memory is considered authoritative. There's no system call to sync in the other direction; that is presumed to be automatic, but apparently in WLS's preferred file system it's not. When you startup LMDB, you create what's called an "environment", which writes an 8K file (two 4K pages) and mmaps it. Then when you create what's called a "database" it writes another 4K page onto the end of the file to store the database's indexing metadata. But due to the aforementioned mmap non-standard file system goofiness, the contents of that third 4K page is not seen in the mmaped address space, so when you try to do a database read it tries to use the indexing info and dies horribly because the page is all zeros.

However, if you write that page to the file via memory writes into the mapped address space, then it all works. LMDB in fact has a mode that's designed to work exactly that way, but it's turned off by default. The reason, apparently, is that LMDB is fundamentally a C/C++ API and the expectation is that you are going to be calling it from C or C++ code (which in fact we are too -- the glue code the realizes the node-lmdb package that presents a JS API is actually written in C++). According to the LMDB folks, not having the write mechanism go through mmap is a defensive measure to protect the integrity of the database, by adding a level of indirection to shield against having the data mangled by bugs that stomp through memory fiddling with pointers in the way that buggy C/C++ code is prone to. However, there's a flag you can set at LMDB environment creation time that says "never mind that, I wanna live dangerously: just write directly onto the mmaped address space". Presuming that the tiny amount of C++ code in the node-lmdb package is correct, using the mmapd pages directly should be fine, since any potential scribbly bugs they are trying to protect us from will be in JavaScript code that is memory safe.

Anyway, when we set that flag, Dean's crash test ran to completion on Dean's system instead of crashing, and all the swing-store-lmdb unit tests passed as well.

It's a single line change to lmdbSwingStore.js. I'll have a PR up shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants