Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rust): Replace Entries by EntriesWithDynamicAdapters #135

Merged

Conversation

Hywan
Copy link
Member

@Hywan Hywan commented Sep 16, 2024

This Entries method is being removed from the FFI API, so I'm updating this project.

This `Entries` method is being removed from the FFI API, so I'm updating
this project.
@Hywan Hywan requested a review from kegsay September 16, 2024 09:50
@Hywan
Copy link
Member Author

Hywan commented Sep 16, 2024

@kegsay Any help 😄? My Go skills are close to zero…

@kegsay
Copy link
Member

kegsay commented Sep 25, 2024

Ah, now you get to see how people actually need to use the FFI API... the problem is a test is failing with:

   thread caused non-unwinding panic. aborting.
  SIGABRT: abort
  PC=0x7f27e6a969fc m=5 sigcode=18446744073709551610
  signal arrived during cgo execution
  
  goroutine 18 [syscall]:
  runtime.cgocall(0xb51c10, 0xc000df6ca8)
  	/opt/hostedtoolcache/go/1.21.13/x64/src/runtime/cgocall.go:157 +0x4b fp=0xc000df6c80 sp=0xc000df6c48 pc=0x427b6b
  github.com/matrix-org/complement-crypto/internal/api/rust/matrix_sdk_ffi._Cfunc_uniffi_matrix_sdk_ffi_fn_free_roomlistentrieswithdynamicadaptersresult(0x7f278c855a00, 0xc000e10360)
  	_cgo_gotypes.go:6100 +0x3f fp=0xc000df6ca8 sp=0xc000df6c80 pc=0x8d955f
  github.com/matrix-org/complement-crypto/internal/api/rust/matrix_sdk_ffi.FfiConverterRoomListEntriesWithDynamicAdaptersResult.Lift.func1.1(0x0?, 0xc000df6d18?)
  	/home/runner/work/complement-crypto/complement-crypto/complement-crypto/internal/api/rust/matrix_sdk_ffi/matrix_sdk_ffi.go:9741 +0x5d fp=0xc000df6ce8 sp=0xc000df6ca8 pc=0x96d37d

This happens when the last reference to a pointer is destroyed, which then causes things to be cleaned up. Why did it happen in that test? Randomly, when the GC decided to do it. This isn't ideal, you should be tidying up as you go along. To do this, you need to call .Destroy() on each value returned from the FFI layer.

@Hywan Hywan marked this pull request as ready for review October 7, 2024 09:09
@Hywan
Copy link
Member Author

Hywan commented Oct 7, 2024

Thanks for your help! I was wondering why the GC wasn't annoying before this PR though 🤔, any, it's great, thanks!

@kegsay kegsay merged commit 9fff69a into matrix-org:main Oct 7, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants