search

salortiz / LMDB_File

Perl wrapper around the OpenLDAP's LMDB
Other
8 stars 12 forks source link

deadlock, Mac OSX #31

Open akotlar opened 5 years ago

akotlar commented 5 years ago

If making many databases, get deadlock (linux error 35), without NOTLS.

Example case attached.

Run on Mac OSX Mojave ( 10.14.0 ).

Could you help me understand whether this is an LMDB or LMDB_File issue? Related issue here: https://github.com/bmatsuo/lmdb-go/issues/94

deadlock.pl.zip

akotlar commented 5 years ago

@hoytech may have a comment

I've forked this lib, and updated the liblmdb dev to the latest commit of LMDB. That appears to fix this issue. Will make a pull request.

hoytech commented 5 years ago

Hey, nope no comment. I haven't noticed this, and I don't have a mac to test on. If upgrading the lib fixes it then great let's do it. Probably good to keep the dependency up to date in general.

I believe Howard Chu has committed to keeping the on-disk format of the DB stable up until at least version 1.0 so shouldn't be any issues there.

salortiz commented 5 years ago

I don't have a Mac either, so can't help. For Github users its documented that the user should update the liblmdb submodule. @hoytech: IMO your Alien::LMDB should be updated for CPAN users.

akotlar commented 5 years ago

Thanks guys. Made a pull request that simply updates the dependency. Not sure if you prefer the git@github.com address; I prefer https for dependencies since this avoids the need to enter a passphrase is you have set one.

hoytech commented 5 years ago

@salortiz - Yes great point, I'll update Alien::LMDB too. 0.9.22 appears to be the latest version, hopefully that has whatever fix is needed.

hoytech commented 5 years ago

OK Alien::LMDB 0.104 has been released to CPAN.

akotlar commented 5 years ago

Thank you both. Original problem seems to remain on Mac OSX, so this is something deeper, and looking through mdb.c, it appears USE_SYSV_SEM was already used for Mac OSX (https://github.com/LMDB/lmdb/commit/2e3eaf2ce26bf643d52c4c98b77b9d9bc71ed2c1). Releasing an updated LMDB_File that contains latest LMDB release seems useful in any case.

akotlar commented 5 years ago

By the way, I noticed that perl Makefile.PL returns the following message on Mac:

"Will use SYSTEM lmdb in " , with the path blank.

This happens because $libdata[3] is "" on OSX. Solution:

warn "Will use SYSTEM lmdb in @{[$libdata[3] || $Head]}\n";

instead of 

warn "Will use SYSTEM lmdb in $libdata[3]\n";

This doesn't I believe affect the library installation, even when installed locally (since after detecting LMDB in system, in WriteMakefile( LIBS => ( INC => $lmdb_dir # 'liblmdb/libraries/liblmdb' ) ).

hoytech commented 5 years ago

Since you're using the github method instead of the CPAN method I'm not sure if this will affect you, but FYI there is a bug in Alien::LMDB where it isn't compatible (or maybe just the tests?) with a system liblmdb. @plicease described the fix here: https://github.com/Perl5-Alien/Alien-Build/issues/69

I'm a bit busy right now, but will try to fix this ASAP.

akotlar commented 5 years ago

Good to know! Thanks for looking into it, no rush, I'm sure you're quite busy. Would this affect "cpanm ." install (I'm not 100% clear on what say doing "cpanm ." does compared to make && make install)?

plicease commented 5 years ago

cpanm and perl Makefile && make && make install would both be effected IF the system liblmdb is installed. The workaround for now is to set ALIEN_INSTALL_TYPE=share, that will force Alien::LMDB to not use the system liblmdb. example:

% env ALIEN_INSTALL_TYPE=share cpanm ALien::LMDB