Skip to content

Package lmdb.1.1.2 - #30198

Open
madroach wants to merge 3 commits into
ocaml:masterfrom
madroach:master
Open

Package lmdb.1.1.2#30198
madroach wants to merge 3 commits into
ocaml:masterfrom
madroach:master

Conversation

@madroach

@madroach madroach commented Jul 5, 2026

Copy link
Copy Markdown
Contributor
  • Switch to build system to ocamlbuild
    • Configuration is now done in myocamlbuild.ml: finding lmdb backend, fallback to shipped backend
    • hopefully more reliable on windows and OSX
  • remove dependency on bigstringaf
  • upgrade shipped backend to 1.0.0
  • add two phase commits and rollback

@madroach
madroach force-pushed the master branch 4 times, most recently from 301c19e to 94d9b48 Compare July 6, 2026 21:49
@avsm

avsm commented Jul 24, 2026

Copy link
Copy Markdown
Member

Is this deliberately a draft PR @madroach?

@madroach

Copy link
Copy Markdown
Contributor Author

Yes. This is not yet ready.

@madroach
madroach force-pushed the master branch 8 times, most recently from 0061e2c to bc08b93 Compare July 29, 2026 07:22
@madroach
madroach marked this pull request as ready for review July 29, 2026 07:39
@madroach

Copy link
Copy Markdown
Contributor Author

Tests on MacOS still fail randomly. No matter whether we use the system lmdb backend version 0.9.35 or the included version 1.0.0. To me this seems to be a bug in the backend or even MacOS.

madroach added 2 commits July 29, 2026 03:08
disabled system lmdb to see whether shipped lmdb 1.0.0 would fix test
failures on MacOS. This did not fix the test failures. Therefore
re-enable depexts on system lmdb.
@madroach

Copy link
Copy Markdown
Contributor Author

Since I don't own any Apple silicone I got an account on macincloud.com. There the tests all pass reliably on Apple M2 macOS Tahoe 26.2. The CI MacOS tests run on amd64. I have no idea about how to look further into this.

@jmid

jmid commented Aug 23, 2026

Copy link
Copy Markdown
Member

I've rerun this and some more pass while others still fail.
https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/b909a37110b3a4e92120dca1ee251ac90d5deccf/variant/macos,macos-homebrew-ocaml-4.14-arm64,lmdb.1.1.2,tests

#=== ERROR while compiling lmdb.1.1.2 =========================================#
# context              2.5.1 | macos/arm64 | ocaml-base-compiler.4.14.4 | pinned(https://github.com/Drup/ocaml-lmdb/archive/refs/tags/1.1.2.tar.gz)
# path                 ~/.opam/4.14.4/.opam-switch/build/lmdb.1.1.2
# command              ~/.opam/opam-init/hooks/sandbox.sh build ocamlbuild test
# exit-code            11
# env-file             ~/.opam/log/lmdb-76753-6e0aaf.env
# output-file          ~/.opam/log/lmdb-76753-6e0aaf.out
### output ###
# + /Users/mac1000/.opam/4.14.4/bin/ocamlc.opt -config
# + ocamlfind ocamlc -config
# + ocamlfind ocamldep -package alcotest -package fmt -modules tests/test.ml > tests/test.ml.depends
# + ocamlfind ocamldep -package alcotest -package fmt -modules tests/pr.ml > tests/pr.ml.depends
# + ocamlfind ocamlc -c -thread -I src -package alcotest -package fmt -w +a-40-42-44-45-70 -I tests -o tests/pr.cmo tests/pr.ml
# + ocamlfind ocamlc -c -thread -I src -package alcotest -package fmt -w +a-40-42-44-45-70 -I tests -o tests/test.cmo tests/test.ml
# + ocamlfind ocamlc -linkpkg -custom -thread -package alcotest -package fmt -I src -I tests src/lmdb.cma tests/pr.cmo tests/test.cmo src/liblmdb_stubs.a -o tests/test.byte
# + tests/test.byte
# + tests/test.byte
# Version: LMDB 1.0.1: (Aug 6, 2026)
# Or: (1,0,1)
# OCaml Version: 4.14.4
# Testing `Lmdb'.
# This run has ID `95C02FRX'.
# 
#   [OK]          capabilities                0   capabilities.
#   [OK]          types                       0   value restriction.
#   [OK]          types                       1   can read from writable.
#   [OK]          types                       2   ro txn on rw env.
#   [OK]          map                         0   add uni.
#   [OK]          map                         1   add dup.
#   [OK]          map                         2   set uni.
#   [OK]          map                         3   set dup.
#   [OK]          map                         4   get uni.
#   [OK]          map                         5   get dup.
#   [OK]          map                         6   dispenser uni.
#   [OK]          map                         7   dispenser_rev uni.
#   [OK]          map                         8   dispenser dup.
#   [OK]          map                         9   dispenser_rev dup.
#   [OK]          map                        10   dispenser_all.
#   [OK]          map                        11   dispenser_rev_all.
#   [OK]          map                        12   remove.
#   [OK]          map                        13   close.
#   [OK]          cursor                      0   fold_left uni.
#   [OK]          cursor                      1   fold_right uni.
#   [OK]          cursor                      2   iter uni.
#   [OK]          cursor                      3   fold_left dup.
#   [OK]          cursor                      4   fold_right dup.
#   [OK]          cursor                      5   iter dup.
#   [OK]          cursor                      6   fold_left_all.
#   [OK]          cursor                      7   fold_right_all.
#   [OK]          cursor                      8   iter_all.
#   [OK]          cursor                      9   add uni.
#   [OK]          cursor                     10   add dup.
#   [OK]          cursor                     11   set uni.
#   [OK]          cursor                     12   set dup.
#   [OK]          cursor                     13   get uni.
#   [OK]          cursor                     14   get dup.
#   [OK]          cursor                     15   remove.
#   [OK]          cursor                     16   walk uni.
#   [OK]          cursor                     17   walk dup.
#   [OK]          cursor                     18   first/last get first/last val...
#   [OK]          cursor                     19   *_all.
#   [OK]          cursor                     20   get_multiple.
# Command got signal -10.

I then looked at the stubs code: https://github.com/Drup/ocaml-lmdb/blob/master/src/lmdb_stubs.c
I don't believe what this is doing is safe: The GC may move things around, so one has to follow a number of rules in order not to mess things up. Failing to do so, the GC may trigger at an unfortunate time, moving a memory chunk, which is then later dereferenced, causing what appears like a random failure.

I recommend https://ocaml.org/manual/5.5/intfc.html#s:c-gc-harmony
Starting with the first one:

Rule 1  A function that has parameters or local variables of type value must begin with a call to one of the CAMLparam macros and return with CAMLreturn, CAMLreturn0, or CAMLreturnT.

I see several functions in https://github.com/Drup/ocaml-lmdb/blob/master/src/lmdb_stubs.c not respecting that.

@madroach

Copy link
Copy Markdown
Contributor Author

@jmid Thanks for trying again and having a look at the stubs. I'm pretty sure I'm handling the GC and allocating stuff correctly. The manual says this:

In some cases, the rules dictate registration of variables that may not be technically needed (eg because the referenced OCaml value is an immediate, or because the GC is guaranteed not to be invoked during the lifetime of the variable).

In most stubs I don't need to register the values because;

  • Many values are immediates (txn, dbi, env, flags).
  • Block values are unpacked before allocations or caml_release_runtime_system() (which may trigger GC runs).
  • No values are accessed after allocation or caml_release_runtime_system().

Do you still see any unsafe omitance of root registrations?

@jmid

jmid commented Aug 24, 2026

Copy link
Copy Markdown
Member

Hm, I've now spent a good chunk of time scratching my head over this stubs code.
The whole hide and unhide seems a bit too clever for me! 😅

Looking at:

CAMLprim value mdbs_init(value unit)
{
  CAMLparam0();
  CAMLlocal4(version, string, array, pair);
  int major, minor, patch;
  unsigned i;

  exn_exists = caml_named_value("LmdbExists");
  exn_map_full = caml_named_value("LmdbMapFull");
  exn_error  = caml_named_value("LmdbError");

  [...]

where the latter looks up values and stores them in global variables.

The manual page says:

The pointer returned by caml_named_value is constant and can safely be cached in a C variable to avoid repeated name lookups. The value pointed to cannot be changed from C. However, it might change during garbage collection, so must always be recomputed at the point of use.

I'm wondering if those globally cached pointers can't be invalidated by an OCaml heap allocation happening before they are read?

This callback function also strikes me as dangerous:

int mdbs_msg_func(const char *msg, void *callback)
{
  int ret;
  caml_acquire_runtime_system();
  ret = Int_val(caml_callback(
	*(value *)callback,
	caml_copy_string(msg)));
  caml_release_runtime_system();
  return ret;
}

caml_copy_string allocates on the OCaml heap, which can trigger a GC that moves things around, which means callback may no longer point to the desired function closure after the caml_copy_string call, AFAICS.

@madroach

Copy link
Copy Markdown
Contributor Author

@jmid Thanks again for looking into this code !

About mdbs_msg_func: Fixed in Drup/ocaml-lmdb@2af1783

caml_named_value returns a constant, unchanging pointer to a value. The value pointed at by this value * may change. The pointer won't change. See the example following the part you quoted from the manual. The manual might be better worded like this:

… so must always be dereferenced at the point of use.

That's what I do in the stubs, too. I store the constant value *, not the volatile values.

The hide() and unhide() hide pointers to outside the OCaml heap from the GC without wrapping them in a custom block. By setting bit 0 the pointers will look like an integer to the GC. This relies on the fact that allocations happen word-aligned and therefore in those pointers the first few bits are always 0. Maybe I should add some comments explaining this.

If you want to continue this conversation, please open an issue at https://github.com/Drup/ocaml-lmdb/issues or just send me an email.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants