Page MenuHomePhabricator

C#: Update C# code-generation to use a new ICustomMarshaler in some string usages that were leaking
ClosedPublic

Authored by lucas on Feb 27 2020, 9:33 AM.

Details

Summary

When C calls a function that return/has an out string and it was overwritten by C# inherit class the C portion
wasn't cleaning its copy. Now, when a C calls a C# delegate function, Strings that are out values or return
values use a new marshaler (specific to this case) that uses Eina short lived strings (Eina_Slstr) instead of
duplicating it with strdup, so at some point, the string passed to C is deleted.

To do so, a direction_context (a new Context at generation_contexts.hh) was created. It is only used when a C#
delegate is being called from C (so this context is only set in function_definition.hh and property_definition.hh,
where it is set to native_to_manage to indicate that it is a native call to a managed function).

When this direction_context is set and the String being marshaled is not marked with an @move tag and it is an
out or return value, the new StringOutMarshaler (implemented at iwrapper.cs) is used (instead of
StringKeepOwnershipMarshaler).

When marshaling a managed data to native this marshaler uses eina short lived string (Eina_Slstr) that will be
automatically deleted. This delete is bounded to "the loop of the current thread or until the clear function is called
explicitly" as said at src/lib/eina/eina_slstr.h.

Test Plan
  • Meson configured with -Dbindings=mono,cxx -Dmono-beta=true, and tests run with ninja test all, to make sure

everything else works.

  • For the leaking test uncomment the for at return_string_from_virtual and out_string_from_virtual methods at

src/tests/efl_mono/Strings.cs test cases;

  • Run valgrind as specified by mono at

https://github.com/mono/website/blob/gh-pages/docs/debug+profile/debug/index.md, using mono.supp available at
https://github.com/mono/mono/blob/master/data/mono.supp. With efl_mono.dll, efl_mono_test.dll and a mono.supp at
the current directory I ran valgrind with efl-mono-suit.exe:

valgrind --tool=memcheck -v --leak-check=full --log-file=log --smc-check=all --leak-check=full --show-leak-kinds=all --suppressions=mono.supp  mono efl-mono-suite.exe

Comparing the log from valgrind at this diff and the one from the master branch, it's visible that there are ~100k more
allocs at the heap, and more ~500KB at exit. Besides that, other values at the heap summary seems statistically equal:

  • Master:
==126683== HEAP SUMMARY:
==126683==     in use at exit: 1,634,589 bytes in 33,474 blocks
==126683==   total heap usage: 1,100,814 allocs, 1,042,563 frees, 425,785,337 bytes allocated
==126683==
==126683== Searching for pointers to 33,797 not-freed blocks
==126683== Checked 25,257,152 bytes
  • From this Diff:
==81715== HEAP SUMMARY:
==81715==     in use at exit: 2,123,170 bytes in 33,959 blocks
==81715==   total heap usage: 1,121,425 allocs, 1,062,689 frees, 426,732,116 bytes allocated
==81715==
==81715== Searching for pointers to 34,282 not-freed blocks
==81715== Checked 25,936,384 bytes

At leak summary there is a significant reduction on definitely lost bytes (~240KB):

  • Master:
==126683== LEAK SUMMARY:
==126683==    definitely lost: 347,087 bytes in 26,670 blocks
==126683==    indirectly lost: 14,632 bytes in 221 blocks
==126683==      possibly lost: 0 bytes in 0 blocks
==126683==    still reachable: 1,189,078 bytes in 6,906 blocks
==126683==         suppressed: 0 bytes in 0 blocks
  • From this Diff:
==81715== LEAK SUMMARY:
==81715==    definitely lost: 107,376 bytes in 6,692 blocks
==81715==    indirectly lost: 14,478 bytes in 221 blocks
==81715==      possibly lost: 0 bytes in 0 blocks
==81715==    still reachable: 1,917,524 bytes in 27,369 blocks
==81715==         suppressed: 0 bytes in 0 blocks

And besides the number of "still reachable" it is statistically equal to master without uncomment the for at
return_string_from_virtual and out_string_from_virtual methods at src/tests/efl_mono/Strings.cs test cases:

  • Master without the test case:
==112684== LEAK SUMMARY:
==112684==    definitely lost: 107,200 bytes in 6,691 blocks
==112684==    indirectly lost: 14,468 bytes in 218 blocks
==112684==      possibly lost: 0 bytes in 0 blocks
==112684==    still reachable: 1,188,177 bytes in 6,878 blocks
==112684==         suppressed: 0 bytes in 0 blocks

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
lucas created this revision.Feb 27 2020, 9:33 AM
lucas requested review of this revision.Feb 27 2020, 9:33 AM
lucas updated this revision to Diff 29260.Feb 28 2020, 7:58 AM
lucas edited the test plan for this revision. (Show Details)
lucas edited the summary of this revision. (Show Details)

Now using eina short lived string instead of idle events to deal with the leak.

felipealmeida accepted this revision.Mar 23 2020, 10:17 AM
This revision is now accepted and ready to land.Mar 23 2020, 10:17 AM
This revision was automatically updated to reflect the committed changes.

I am getting new warnings after this patch:

marshall_annotation.hh:66:20: warning: ‘if constexpr’ only available with -std=c++1z or -std=gnu++1z`
marshall_annotation.hh:110:20: warning: ‘if constexpr’ only available with -std=c++1z or -std=gnu++1z
lucas added a comment.Apr 1 2020, 2:29 PM

@segfaultxavi D11638 should deal with it.