libplctag / libplctag.NET

A .NET wrapper for libplctag.
https://libplctag.github.io/
Mozilla Public License 2.0
217 stars 55 forks source link

Can .Net free unmanaged memory? #105

Closed kyle-github closed 3 years ago

kyle-github commented 3 years ago

Sorry guys, dumb question: I am really close to figuring out how to dispose of C-style string data allocated in the native DLL for Java and I have been poking around a bit but my Google fu is weak today; can .Net do this too?

The reason I am asking is that I think I can make a C string API that is much, much easier to use than what I came up with before. But it requires that I return a newly allocated C string. Here is what the new string API would look like:

char *plc_tag_get_string(int32_t tag_id, int string_start_offset);
int plc_tag_set_string(int32_t tag_id, int string_start_offset, const char *string_val);
int plc_tag_get_string_capacity(int32_t tag_id, int string_start_offset);
int plc_tag_get_string_total_length(int32_t tag_id, int string_start_offset);

It is that first function that is the problem child. I have to allocate memory in order for this to really be easy to use. But then the caller needs to free it. I am about 99% certain that I have figured out how to do this in Java, I want to make sure that it can be done in .Net.

Can it be done without both of you tearing your hair out?

timyhac commented 3 years ago

From what I've seen with other P/Invoke methods, it would need to ask the caller to allocate the memory, a reference to that memory is passed to the native library, which then the fills it with data.

For some reason, one of the applications I maintain uses GetPrivateProfileString to retrieve settings from an INI file. This site (which seems to be the nearly-official reference for how to interface with Win32 functions via .NET) says that this function takes a StringBuilder object (which is closer to a C-style string, strings in .NET are immutable, whereas StringBuilder is the mutable version), which GetPrivateProfileString fills with data.

How would feel about this approach?


Off the top of my head I don't know how to free unmanaged memory from .NET, but if it can be done in Java it's pretty likely there would be something similar in .NET.

kyle-github commented 3 years ago

It might be as simple as calling the native free() function.

kyle-github commented 3 years ago

The problem with passing in a buffer is (well there are two):

  1. You will need to get the string size before you pass in the buffer. Otherwise you will not know how big to make the buffer.
  2. GC makes things fairly complicated. You might need to pin the buffer memory so that GC does not move it while the C code is running. Mostly the FFI layers take care of this, but only when you pass a string. But that is not a buffer as string are immutable.

That was where I was running aground before. It is definitely easier to pass in a buffer, but just a little extra effort (at least in Java) seems to make the API so much easier to use.

timyhac commented 3 years ago

Yep OK I see where we're going with this.

Were you thinking that you would make another API for the call to free() - something like plc_tag_free_string(char* ptr), or that we free the unmanaged memory via a .NET API? How do you do it in Java?

kyle-github commented 3 years ago

In Java JNA, you can just call Native.free(strPtr) and the memory goes away. I think there is a way to make it do this automatically when JNA converts the C char * to a Java String. It works just fine for the strings I return from plc_tag_decode_error() which are owned by the core library (static strings).

I could make another API call to wrap the C library free() function, but usually the C library functions are already available in one form or another.

I am going to ask on the JNA forum/list what the most idiomatic way to handle this is. I would assume that C# is even better because it generally has a better interop story than Java.

timyhac commented 3 years ago

I'm not sure I'm the right person to comment on how free() could be invoked directly. I've done a little bit of research on google and a book, probably much of what you've done, and mostly found "not possible"s.)

But! This looks promising: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.freehglobal?view=netstandard-2.0 It seems to invoke the operating system's memory management API, so while dangerous, is exactly what we need in this scenario.

I think it is the equivalent of JNA's Native.free() you mention.

Relevant StackOverflow posts:

While it does seem to be possible, the articles that seemed the most authoritative indicated that it would be preferable to have each component handle its own memory.

Probably we should look at how other wrappers would handle this too.

kyle-github commented 3 years ago

I found and read through some of the same things. It looks like the answer is actually "no". I will need to expose another library function to wrap free :-(

I cannot call the OS function because that does not correctly handle memory allocated with the C runtime library (malloc(), calloc() and friends). Apparently MS did not make them compatible in any way. So you cannot use OS functions to free memory allocated in C. Grrr...

OK. Time to rethink the API again.

kyle-github commented 3 years ago

Keith (from the Julia wrapper) also noted that split responsibility of the memory buffer is bad coding etiquette. So how about one of these?

Perhaps put all the memory handling in the wrapper?

int plc_tag_get_string(int32_t tag_id, int string_start_offset, char *buffer, int buffer_size);
int plc_tag_set_string(int32_t tag_id, int string_start_offset, const char *string_val);
int plc_tag_get_string_capacity(int32_t tag_id, int string_start_offset);
int plc_tag_get_string_total_length(int32_t tag_id, int string_start_offset);

First you call plc_tag_get_string_capacity() to get the maximum capacity. Then you create a byte buffer sufficiently large to handle that. Then you call plc_tag_get_string() with that buffer. You get back a value that is the length of the string, or a negative number that is an error (i.e. PLCTAG_ERR_TOO_SMALL if there is insufficient space).

Another option is to expose a free function:

char *plc_tag_get_string(int32_t tag_id, int string_start_offset);
int plc_tag_free_string_buffer(char *buffer);
int plc_tag_set_string(int32_t tag_id, int string_start_offset, const char *string_val);
int plc_tag_get_string_capacity(int32_t tag_id, int string_start_offset);
int plc_tag_get_string_total_length(int32_t tag_id, int string_start_offset);

That allow you to free without knowing how long the string is in advance. But you still need to call plc_tag_get_string() capacity and allocate a buffer when you write a string. I think that argues for the first version since you have to write that code anyway for the write case.

kyle-github commented 3 years ago

Another problem with the second set of API functions (with free) is that memory handling is inconsistent. In the case you read, the library creates and fills the string, but the application/wrapper still needs to determine when to free it. In the case you write a string, the application/wrapper is solely responsible.

The first option does not have this problem. And, as Keith pointed out in the Julia issue I raised for this, then the buffer could be reused on the wrapper side.

timyhac commented 3 years ago

Alrighty, are you OK to close this?

kyle-github commented 3 years ago

As surfaced in the .Net wrapper discussion, setting one byte at a time to zero out the remaining bytes of a string might be fairly slow/heavy on platforms with unoptimized mutexes. So add a function to the API to set a range of bytes just like memset(). That makes the final string API this:

/* base string API */
int plc_tag_get_string(int32_t tag_id, int string_start_offset, char *buffer);
int plc_tag_set_string(int32_t tag_id, int string_start_offset, const char *string_val);
int plc_tag_get_string_capacity(int32_t tag_id, int string_start_offset);
int plc_tag_get_string_total_length(int32_t tag_id, int string_start_offset);

/* byte range/raw byte API */
int plc_tag_clear_bytes(int32_t tag_id, int start_offset, int range_length, uint8 val);
int plc_tag_get_bytes(int32_t tag_id, int start_offset, int range_length, uint8_t *destination_buffer);
int plc_tag_set_bytes(int32_t tag_id, int start_offset, int range_length, uint8_t *source_buffer);

To be continued...

kyle-github commented 3 years ago

This works for all the cases I could come up with.

timyhac commented 3 years ago

Further conversation about the core library string API should be moved to the core library repo.