Segfault on close with pending put #157
Comments
|
I also saw the following core dump: *** Error in `node': free(): invalid pointer: 0x00007f3cbc0002b8 ***
Aborted (core dumped)I am running Ubuntu 14.04 on an Intel x64. |
|
can you try |
|
I tried using TAP version 13
# can push values through a level database
Segmentation fault (core dumped)I suspect db.close(function(er) {
if (er) throw er;
t.end();
})is triggering the issue because when I take it out, I don't get a segfault. |
|
the segfault is probably cause by still open iterators. i think @kesla worked on this? |
|
Yea, I took a peak at the source code and thought I'd try waiting like so: dbify.on('end', function() {
db.close(function(er) {
if (er) throw er;
t.end();
})
})
dbify.end();Since I started waiting, I haven't received a segfault. Was it rude how I was shutting down? I am new to level and it hadn't occurred to me to do this. |
|
@lakowske Nope you weren't rude, this is the way it's supposed to work. There's some race condition happening somewhere. I'll dive into it and see if I can find something :) |
|
@lakowske I'm wondering if the stream haven't finished yet and is trying to do |
|
I'm not sure how to test if the stream haven't finished yet, but I tried anyway. Here is what I tried: test('can push values through a level database', function(t) {
var db = level('./test.db');
var dbify = push(db);
var stringify = JSONStream.stringify(false);
dbify.pipe(stringify).pipe(process.stdout);
dbify.write({key:'hi', value:"wisconsin"});
db.get('hi', function(error, value) {
console.log(value);
})
db.close(function(er) {
if (er) throw er;
t.end();
})
})and got the following: TAP version 13
# can push values through a level database
wisconsin
Segmentation fault (core dumped)Not sure if that helps, but I have to get to work now. |
|
Ok, so I think I found something. It seems it's related to |
|
Tried to strip this sample into something more minimalistic to see if we can reproduce the same kind of error without involving streams and other modules. This seems to segfault as well. I was using var level = require('level')
var db = level('./test.db')
db.put('foo', 'bar', { sync: true }, function (err) {
console.log('back from put', err)
})
db.close(function (err) {
console.log('closed the db', err)
}) |
ralphtheninja
added
bug
|
I can confirm that this segfaults on |
|
@ralphtheninja Over lunch I tried your minimalistic version without sync, so: var level = require('level')
var db = level('./test.db')
db.put('foo', 'bar', function (err) {
console.log('back from put', err)
})
db.close(function (err) {
console.log('closed the db', err)
})and got the following on my third run: Segmentation fault: 11This occurred on OS X 10.9.5 (FWIW). |
|
My guess is that there's an open iterator that's not being closed properly - but I don't have any pointers on where to look really to figure it out. Sorry. |
|
My gut is that it is my fault, I probably got some bug in updating leveldown 0.10 to node v0.12 and iojs. Basically it's time for lldb/gdb and figuring out what the problem is :(. |
|
It's impossible for me to reproduce without |
|
Hey @mcollina Using https://github.com/ddopson/node-segfault-handler |
|
Dived in a little more this morning and and I think @kesla may have a point. https://github.com/rvagg/node-leveldown/blob/v0.10.4/src/database.cc#L256 My next step would be to see if writebatch in Leveldb
Also note that this is presenting like a race condition. |
|
@No9 Yes I've noticed that it hangs as well. Just doesn't end. And on ubuntu I eventually get the system program error dialog:
|
|
Ok, so I did the following: And once in gdb: So now we have at least file names and file numbers. |
|
@mcollina I don't think this is something related to your changes :) |
|
👯! Big question: is this fixed in 1.0.0? |
|
@mcollina Nope (I just tested it) |
|
Unfortunately I have no time now :(, but it is something I would love to solve :). |
|
We should solve this before releasing |
|
Updated stack trace with |
|
Implemented a writing guard for LevelUP here: Level/levelup#338 |
|
btw: #174 ended up not fixing the bug |
|
The |
|
I just ran var levelup = require('levelup');
var leveldown = require('leveldown');
var db = levelup('./test.db');
db.put('foo', 'bar', { sync: true }, function (err) {
console.log('back from put', err);
});
db.close(function (err) {
console.log('closed the db', err);
});on |
|
I ran into segmentation fault when I'm using leveldown v2.0. Try to upgrade to leveldown v4 to see if it will still exist. Related to: |
|
After I switched to the @snowyu Would you like to merge back what you did for fix this issue back to the leveldown repository? Because I believe it will be very valuable for all the users. |
|
In fact I had already pull-requested it (synchronus methods) as early as 4 years ago #136 . |
|
Oh, it's a very old and long thread, it takes some time to read. It seems the community is not interested in the sync api, however, a segment fault fix should be welcome very much. To make it simple, could you please just pull request the segment fault fix so that we can see if we can fix level down without changes other api? It would be awesome if we could, thank you very much. |
|
Nohope, I've changed the low-level core totally using the sync methods only in it. In my opinion. KISS is more important. Now it is clear and easy to read and debug. And nodejs has supported the worker threads since version 10.5.0 ( Why not focus on the database business with all your heart and soul? Let the nodejs to do other things(coroute/thread/process schedule). Is the sync way real dangerous performance killer? |
|
Understood. So your fix was based on the Sync modification right? That will be a pity that we can not get this fixed merged. And about the sync API: I have no bias about each side. However, I have to agree that if we use a Sync API, the event loop has to wait for the sync API to finish before it can do any other jobs. I believe that's why the people do not like the sync API. |
|
Yes. I do not think you understand my words. First you can run it in a worker thread(or generator?) to avoid the block. |
|
nosql-leveldb works like a charm in my environment, I'm using it for my Wechat Bot as local file cache behind Protocol Payloads. About the |
|
The concept is put it(a task) into another process/thread/coroutine(fiber) to run.
|
|
Thanks for the information. However, it seems that the Let's make the problem simple: If we have the following codes to run, how can we guarantee the function block () {
let i = 0;
for (i = 0; i < 10000000000; i++) {
let j = i
j = (i + j) / (i * j)
}
}
function main () {
setInterval(() => { console.log('setInterval runned') }, 100)
block()
}
main()The above code will wait around 10 seconds before output |
|
The thread way is the leveldown internal used(a new thread created each async operation). The cost of thread/process switching is quite expensive. This is why the performance of synchronization is better than asynchronous here(Asynchronous is roughly twice as slow as synchronization). You can run the benchmark in the node-nosql-leveldb/bench. The coroutine/fiber is a light-weight implementation of task schedule in one thread/process. It uses cooperative scheduling. Assuming that the thread switching is 10 units of time, the coroutine overhead may be just 1. I could yield it when using disk I/O If I completely write the leveldb with js. But it is just an adapter for leveldb library. And I do not think the leveldb is disk I/O intensive in the general scenario. The table, block cache and write buffer are used in leveldb. So unless the very special scene makes the cache miss. You should test(benchmark) your user scene to determine what's your bottleneck. I do not know your idea. if you know wanna howto write |
|
If I understand right, we are simulating Async operations for leveldown because internally leveldb is sync. Thanks for your explanation, I'll look into it later. |
vweevers
changed the title
I am getting a segfault when I run the following unit test. The segfault doesn't happen every time I run the test. Any ideas?
The text was updated successfully, but these errors were encountered: