Comments (33)
Same thing in geoip.Region
var regiondb = new geoipmodule.Region('/tmp/GeoIPRegion.dat')
var foo;
var st = new Date();for(i=0;i<10000000;i++) { foo = regiondb.lookupSync("107.20.181.99") };console.log(new Date() - st)
Run the last line a few times... and you see the memory usage of node process only go up...
FWIW im on v0.4.6
from geoip.
sh*t, I don't know how to fix it......
from geoip.
Is this issue resolved ?
from geoip.
Sorry, not yet.
This probably need pretty heavy changes from current codes. Unfortunately, I am busying for money now.
Maybe next week I'll have some free time, but not sure.
from geoip.
I understand, I would like use it for my project but I need something tested and stable, your geop library looks the most efficient and cleanest by far. Just need stability.
I will watch your repo and hopefully you find some time :)
Thanks
from geoip.
thx, but to be honest, for me, this project is just a platform for learning code on node and some open source project experience.
I will try my best to make it better over time, but not guarantee it will be totally production ready.
If you really need something very very stable, probably you can choose some alternatives.
from geoip.
deleted previous comment, didnt realized my pull was merged.... my bad
from geoip.
Hey Sajal, did you say you have fixed the memory leaks in your pull
from geoip.
I just push some changes in master branch:
Can you guy give it a try, and return some feedback about memory leak.
thanks.
from geoip.
It seems there still some memory problems.
This is memory testing report for 0.4.6
https://gist.github.com/4333807
This is memory testing report for 0.4.7pre
https://gist.github.com/4333801
from geoip.
Some of the leaks are from node.js - you can ignore those.
The following leak still needs to be fixed but isn't a huge problem since the allocation only happens once. You just need to call GeoIP_delete(db);
in the City
destructor.
==1525== 201 (40 direct, 161 indirect) bytes in 1 blocks are definitely lost in loss record 20 of 29
==1525== at 0x4C2B1C7: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1525== by 0x703405C: geoip::City::New(v8::Arguments const&) (in /home/vagrant/GeoIP/build/Release/geoip.node)
==1525== by 0x6CB766: ??? (in /usr/bin/nodejs)
Have a look at this commit from our fork which fixed the lookupSync()
leaks.
As for the Mismatched free() / delete / delete [] error, try replacing delete name
with free(name)
const char * name = _GeoIP_iso_8859_1__utf8(record->city);
data->Set(String::NewSymbol("city"), String::New(name));
delete name; //replace with free(name);
from geoip.
Hey, I test your code, it seems the memory leak is gone (at least in valgrind). awesome !
Can you make a pull request?
from geoip.
@kuno I explained in the initial bug report why I didn't submit a pull request. My commit only fixes leaks in lookupSync()
since this is the only function that we rely on. All of the other async/sync variations using other databases will still leak. I don't have access to other GeoIP databases so I can't fix it for you.
I also explained how to close the leaks in the initial report
- Call
free()
after you're done with the result of_GeoIP_iso_8859_1__utf8()
- Call
GeoIPRecord_delete(record)
when you're done with a record - Call
GeoIP_delete(db)
when you're finished with the database (usually in the destructor)
from geoip.
Thanks, I follow the way you suggested.
It seems the most memory are gong, but seem like the memory leak when new a object still occurs.
Here is the report, https://gist.github.com/4357339.
Anyway, I decide to release current code as 0.4.7, as it will be better than 0.4.6 for most user.
from geoip.
I made some change to code, make this module self-hosted. When I use valgrind to test memory leak, it seem there are no leak any more. But I am not sure...
See, https://gist.github.com/kuno/4357339
Can you help me to confirm that?
from geoip.
A few issues I can see (looking at city.cc only). On these lines you free the same memory twice. You need to remove the delete
call.
GeoIP_delete(c->db); // free()'s the gi reference & closes its fd
delete c->db;
Another thing, if a library is allocating memory and returning a pointer then you need to free the memory with the equivalent free function provided by the library.
I see a few cases where you call GeoIP_record_by_ipnum(c->db, ipnum);
and then later free the pointer with delete record;
. This is only working because they're using the same memory allocator, but keep in mind that the pointer might point to a struct with additional allocations. You need to replace the delete
with GeoIPRecord_delete(record);
.
from geoip.
@kuno we're running a fork of the library (https://github.com/sydneystockholm/GeoIP) with everything stripped out except for city.lookupSync
.
Have a look at the code here => https://github.com/sydneystockholm/GeoIP/blob/master/src/city.cc
from geoip.
Thanks for your suggestions, what about the situation of memory leak, as you might saw?
from geoip.
@kuno it's missing the summary output from valgrind?
from geoip.
Yeah, it is a bit strange for me, too.
But it is actually no summary output from valgrind..., I am doing something wrong?
from geoip.
@kuno Are you running a mac? I've had a similar issue before. Try running through linux if you can
from geoip.
No, I am running in a ubuntu 12.04 virtualbox vm.
It works fine previously.
from geoip.
Did you saw some gdb
, vgdb
things in the output ? I suspect this might be the reason of what we saw?
from geoip.
@kuno Ok, not sure then. Does echo $?
show valgrind exiting with 0?
from geoip.
@kuno The vgdb
thing lets you attach a gdb process for debugging - just ignore it
from geoip.
Yes, echo $?
show 0
after running valgrind --leak-check=full -v test/memory_leak.js
from geoip.
@kuno No clue then, sorry
from geoip.
Anyway, thx
from geoip.
Do I need manually delete a pointer of instance it self, like:
City6 * c = ObjectWrap::Unwrap<geoip::City6>(args.This());
delete c; // ???
or v8's garbage collector will do the job?
from geoip.
@kuno don't delete it. You're getting a reference to the pointer when you need it but you want to keep it around (e.g. if you're calling city.lookupSync() you want the pointer to be around when you call city.lookupSync() again). It should be the destructors job (~City()
) to cleanup the pointer when the object is being collected.
from geoip.
@kuno Send me an email directly if you want more help, [email protected]
from geoip.
Ok, thanks. very helpful.
from geoip.
Closing.
from geoip.
Related Issues (20)
- installation problema HOT 7
- Bug in the async country lookup method HOT 7
- Build fails on Windows 7 HOT 13
- nan.h error on node v0.10.22 HOT 14
- GeoIP on Ubuntu 13.10 HOT 7
- installation problem HOT 2
- core dump when invalid data file supplied HOT 2
- build fails under node v0.11.13 HOT 2
- Compile for io.js 1.0.4 HOT 2
- nan ~1.8.4 required for io.js 2.x.x compatibility HOT 1
- Synchronous method(the recommended way). why? HOT 1
- Installation on CentOs
- Build failed on io.js v2.3.1 HOT 1
- IP2Location LITE Support HOT 1
- use of static to get the value of info HOT 2
- OS X 10.11 Installation Issues HOT 2
- Doesn't build with nodejs 6.1.0 HOT 3
- Is it possible to use GeoIP2-City.mmdb ?
- Bug report(Mac os X 10.12) HOT 1
- Respect DNT
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from geoip.