Foreign interface: BUF_DISCARDABLE seems to be non-working

Stress-testing of my version of Lesta’s C# wrapper for Prolog foreign interface revealed somtheing strage. Repeated call to PL_get_wchars eventually produce an error:

SWI-Prolog: [FATAL ERROR: at Sun Sep 29 18:13:27 2024
Too many stacked strings]

First, I tried by wrapping calls into foreign frame fuctions as described here: SWI-Prolog -- Discarding Data

It didn’t work at all, which is strange by itself.

Then I’ve tried to use BUF_DISCARDABLE with PL_get_wchars, which also didn’t work.

I looked through the code of PL_get_wchars and internal call to PL_save_text and failed to find codepath that uses BUF_DISCARDABLE at all. It seems to check of BUF_MALLOC and fallback to BUF_STACK algorithm otherwise.

Documentation for PL_get_chars contains very interestring framgent for other flag:
CVT_WRITE
Convert any term that is not converted by any of the other flags using write/1. If no BUF_* is provided, BUF_STACK is implied.

But BUF_DISCARDABLE is defined as 0x0, so providing it equals to not providing BUF_* at all. Very confusing.

So, open questions are:

  1. Why using and discarding frames did’t prevent strings stack leackage?
  2. How to use BUF_DISCARDABLE correctly with BUF_DISCARDABLE?

Surely, the answer to the first is in SWI-Prolog -- String buffering. I’m not sure about the current state of BUF_DISCARDABLE. I think you can still use it and it may avoid copying in some cases, probably in particular getting the C text from a Prolog string if the representation matches. If there is no C string with matching representation, it will use BUF_STACK. That will run out if you do not take precautions. Older versions used a ring, but that could result in rather hard to track bugs.

And no, the foreign frame interface only deals with term_t handles and backtracking.

Guess the docs need some work :frowning:

BUF_DISCARDABLE is defined as 0, so it’s the default, isn’t it?

The C++ API uses PL_get_chars() (and wchars) in a few places, and I think the test cases exercise that. Also, PlTerm::get_nchars() explicitly turns off BUF_STACK|BUF_MALLOC|BUF_ALLOW_STACK and it works AFAICT (but inside a string buffer).

Do I need to revisit that code?

I think all should be fine as long as the code is between PL_STRINGS_MARK()/PL_STRINGS_RELEASE(). Strings will be stacked if a copy needs to be made to create a different representation. You only get the native representation if you ask for an ISO_LATIN_1 string from a string or atom or you ask for a wide string from a string or atom that happens to contain code points >255. In all other cases, the call will create a string for you and stores that on the string stack. The two above cases are the current situation, there is still a plan to replace the double representation by UTF-8, which would mean getting UTF-8 and ASCII are without copying.

There appear to be some errors in the C++ API, at least with calls to PL_get_chars(). I’ll try to do a cleanup (and double-check the calls to PL_get_nchars() and PL_get_wchars()), but it might be a few weeks before I can do that.

@AlexeyKosarchuk – I’m curious to see what the C# wrapper looks like, to see if any of its design decisions would improve the C++ wrapper … where can I see the C# wrapper? (The links at SWI-Prolog interface to C# and F# didn’t work for me)

I did a search for PL_*() functions that update a char* or wchar_t* and found the following that are not documented as needing PL_STRINGS_MARK … can any of them generate a temporary copy? (If so, I’ll update the documentation and the C++ API)

PL_atom_mbchars(atom_t a, size_t *len, char **s, unsigned int flags)
PL_get_atom_chars(term_t t, char **a)
PL_get_atom_nchars(term_t t, size_t *len, char **a)
PL_get_string(term_t t, char **s, size_t *len) // DEPRECATED
PL_get_list_chars(term_t l, char **s, unsigned int flags)
PL_get_list_nchars(term_t l, size_t *len, char **s, unsigned int flags)
PL_get_file_name(term_t n, char **name, int flags)
PL_get_file_nameW(term_t n, wchar_t **name, int flags)
PL_cvt_i_string(term_t p, char **c)
PL_cvt_i_codes(term_t p, char **c)

And here are the functions that are documented as requiring PL_STRINGS_MARK:

PL_get_chars(term_t t, char **s, unsigned int flags)
PL_get_nchars(term_t t, size_t *len, char **s, unsigned int flags)
PL_get_wchars(term_t l, size_t *length, pl_wchar_t **s, unsigned flags)

Note that the C++ API tries to avoid the need for PL_STRINGS_MARK (which it abstracts as the RAII class PlStringBuffers) by returning std::string rather than char*.

Here: Алексей Косарчук / swipl-cs-2 · GitLab
Requires Visual Studio 22 & .NET SDK 6.
Tested for 64 bit.

It is my fork from old Lesta’s C# wrapper you’ve mentioned here.
It basicly works, but I still have some leaks and (rarely) random crashes.

I solved major string stack leakage by using BUF_MALLOC and PL_Free after unmarshaling strings, but somwhere strings are still stacked and under stress-testing I still manage to get “too many stacked strings”

Good to hear we will have a functioning C# binding again :slight_smile:

Possibly you found a bug. If you can reduce this to specific API calls, I’m happy to have a look.

It is wise to use the stacked string API though as that avoids a lot of malloc()/free() calls. This saves time and memory fragmentation. Note that while the PL_STRINGS_MARK()/PL_STRINGS_RELEASE() API is C specific, but the underlying PL_mark_string_buffers() and PL_release_string_buffers_from_mark() are not.

PL_mark_string_buffers() is side-effect free. If you call it before and after some operation, its state should not change.

You can also use

 :- set_prolog_flag(string_stack_tripwire, 10).

to get errors if the stack accumulates only 10 stacks. 10 is enough for the built-in predicates. That should allow for reproducing with less stress :slight_smile:

Thanks, I will investigate further and try to distill reproduction to minimal program.

Okay. it seems that careful usage of mark/release strings solved the problem.

The catch was that even if called with BUF_MALLOC PL_get_wchars allocated strings on stack.
Possibly it was some internal intermediate results.

PL_atom_wchars also requires usage of mark/release.

With these fixes and careful usage of frames my code runs indefinitely with no increase of memory footprint and prolog errors.

1 Like

I’ve been reading the code at PL_get_nchars(), which calls PL_get_text(), PL_mb_text(), etc. … It seems that if you know that the value is pure ASCII or not a bignum, then it’s safe to not surround with PL_STRINGS_MARK()/PL_STRINGS_RELEASE() but otherwise, some values might be allocated on the buffer stack (it’s not clear to me when BUF_MALLOC is used – there appear to be places where the flags are ignored and BUF_STACK is used).

@jan – should all calls to the PL_*() functions that pass a pointer to char* be surrounded with mark/release?
If you don’t know off-hand, then I’ll read the code and update the docs with what I find.

It you use BUF_MALLOC(), this should not be needed. The examples by @AlexeyKosarchuk seem to indicate a bug. BUF_STACK surely needs it :slight_smile: BUF_DISCARDABLE may need it, i.e., under some conditions you can do without but typically you’ll need it. It could be that you get a pointer to a fragile char*, such as a Prolog string on the stacks. Almost any stack related API call may cause a garbage collection or stack shift, invalidating this pointer. So, use one of

  • BUF_MALLOC and PL_free()
  • BUF_STACK and PL_STRINGS_MARK()/PL_STRINGS_RELEASE(). Note that this is typically more efficient as small strings do not use malloc()/free(). It will also use a direct pointer to Prolog’s internal when safe (for example when getting the data from an atom)
  • BUF_DISCARDABLE and PL_STRINGS_MARK()/PL_STRINGS_RELEASE(). This is similar than the above, but you must consume the data before making any other Prolog API call (to keep it simple, some are in fact safe). It may avoid a copy compared to BUF_STACK.

I think this needs some documentation updates and a fix for the first, which seems to be not always satisfied.

Just to clarify a bit: if I use PL_get_wchars with BUF_MALLOC, then result is actually located on the heap and is correctly freed by PL_free. But stacked strings still appear somewhere under the hood and without mark/release stack runs out.

Yes, and that should not be the case :frowning:

1 Like

Indeed. Fixed.

This one is documented to use the string stack. It has no argument to specify how the result must be buffered, so that is it. I think this should be considered deprecated. PL_get_wchars() can do the same for you. PL_atom_wchars() is an older API.

1 Like

It appears that some of the code paths can temporarily use the string stack, even if BUF_MALLOC is specified. Maybe this is guaranteed to be safe because it’s guaranteed to be only one string? - but it seems that there could be leaks if mark/release stack isn’t used.

See my patch to PL_get_wchars(). If BUF_MALLOC is specified and we cannot otherwise guarantee there are no stacked intermediate results, the implementation should internally use the stack interface to guarantee no stacked strings are left.

I was thinking about the various calls to findBuffer(BUF_STACK), regardless of what the flags are (mostly in os/pl-text.c but also in other places).

PL_get_wchars() uses one of these. I now fixed that using a mark/release when BUF_MALLOC is requested, but possibly there is a better route by mallocing the value earlier during the conversion. Roughly, we need three steps:

  • Get a string from the Prolog term. This may or may not require buffering.
  • Get it in the right representation (ISO latin 1, UTF-8, locale multibyte or wchar_t*).
  • Get it stored in the right way.

Preferably we want to avoid as much as possible malloc() as well as copying. Might need some reviewing …

Shouldn’t if(flags&BUF_MALLOC) be if(flags&~BUF_STACK)? (to include BUF_DISCARDABLE)

So, can PL_get_text() assume it’s inside a PL_mark_string_buffers()? And this is also true for other functions in pl-text.c, such as PL_mb_text(), PL_text_recode(), etc.?