Possible bug in library persistency?

Help!
1

I have this code in a file 001.pl:

#!/usr/local/bin/swipl -s

:- use_module(library(persistency)).

:- persistent m:p(x:any).

part1 :- m:( db_attach( ‘db.txt’,), assert_p(1)).

part2 :- m:( db_detach, assert_p(2)).

And how I use it:

$ ./001.pl
Welcome to SWI-Prolog (threaded, 64 bits, version 9.3.20-DIRTY)
SWI-Prolog comes with ABSOLUTELY NO WARRANTY. This is free software.
Please run ?- license. for legal details.

For online help and background, visit https://www.swi-prolog.org
For built-in help, use ?- help(Topic). or ?- apropos(Word).

?- part1.
true.

$ cat db.txt
created(1757350507.4964447).
assert(p(1)).

?- part2.
ERROR: db_file `m’ does not exist
ERROR: In:
ERROR: [20] throw(error(existence_error(db_file,m),_170))
ERROR: [18] persistency:persistent(m,assert(p(2))) at /usr/local/lib/swipl/library/persistency.pl:471
ERROR: [16]
ERROR: [15] with_mutex(‘$persistency’,db_assert_sync(m: …))
ERROR: [11] toplevel_call(user:user:part2) at /usr/local/lib/swipl/boot/toplevel.pl:1319
ERROR:
ERROR: Note: some frames are missing due to last-call optimization.
ERROR: Re-run your program in debug mode (:- debug.) to get more detail.
?- m:p(X).
X = 2.

After db_detach, p(X) is empty, as expected, a subsequent assert_p(2)) raises an exception but fills the 2 into p(X).

I think this is a bug. It should not fill in something.

Thanks in advance, Frank Schwidom

2

Indeed, the culprit appears to be in persistency:db_assert_sync/1, where a plain assert gets issued before the persistency’s own assert, which in turn may simply throw (see the quoted session at the bottom).

And, after also re-reading the docs, I would concur with you: that there is a bug there, most probably not only in that method but in all similar ones; and that the docs also could be strengthened, in particular what happens at detach is not precisely stated, whether the assert/retract_xyz get abolished or not: they apparently don’t, but they should IMO.

In general, I’d expect any access to a persistency db to throw if no file is attached, plus there are things that can go wrong when accessing a file anyway, so some kind of exception handling, maybe in the spirit of overall guaranteeing atomicity of operations (i.e. either it all succeeds or it all fails), might be a good thing.

Welcome to SWI-Prolog (threaded, 64 bits, version 9.2.7) [...]

?- debug.
true.

[debug]  ?- % consult
%  library(persistency) compiled into persistency 0.03 sec, 67 clauses

[debug]  ?- m:p(X).
false.

[debug]  ?- part1.
true.

[debug]  ?- m:p(X).
X = 1.

[debug]  ?- part2.
ERROR: db_file `m' does not exist
ERROR: In:
ERROR:   [20] throw(error(existence_error(db_file,m),_188))
ERROR:   [19] error:existence_error(db_file,m) at <redacted>/swipl/library/error.pl:115
ERROR:   [18] persistency:persistent(m,assert(p(2))) at <redacted>/swipl/library/persistency.pl:471
ERROR:   [17] persistency:db_assert_sync(m:p(2)) at <redacted>/swipl/library/persistency.pl:458
ERROR:   [16] <meta call>
ERROR:   [15] with_mutex('$persistency',db_assert_sync(m: ...)) <foreign>
ERROR:   [14] persistency:db_assert(m:p(2)) at <redacted>/swipl/library/persistency.pl:451
ERROR:   [13] m:assert_p(2) at <redacted>/sw03.pl:3
ERROR:   [12] part2 at <redacted>/sw03.pl:7
ERROR:   [11] toplevel_call(user:user:part2) at <redacted>/swipl/boot/toplevel.pl:1317
   Exception: (19) error:existence_error(db_file,m) ? abort
% Execution Aborted

[debug]  ?- m:p(X).
X = 2.

[debug]  ?- listing(p).
:- dynamic m:p/1.

m:p(2).

true.

[debug]  ?- listing(assert_p).
m:assert_p(A) :-
    user:
    (   true,
        persistency:db_assert(m:p(A))
    ).

true.

[debug]  ?- listing(db_assert).
:- public persistency:db_assert/1.

persistency:db_assert(Term) :-
    with_mutex('$persistency', db_assert_sync(Term)).

true.

[debug]  ?- listing(db_assert_sync).
persistency:db_assert_sync(Module:Term) :-
    assert(Module:Term),
    ( persistent(Module, assert(Term))
    ).

true.
3

These are IMO not really bugs, just invalid usage that is not handled properly. It is an old library … But yes, it is still relevant. If anyone wishes to update the docs and/or improve error handling, please file a PR. Please note that such improvements should not cause significant slow down or make the code too hard to read.

Note that the ordering is hard. First assert/1, then update the journal has the advantage that it keeps the journal consistent if a limit or permission causes the assert to raise an error. Note that it might work safely by using transaction/1. Not sure how much impact that has on performance, but probably not too much.

1 Like