-
-
Notifications
You must be signed in to change notification settings - Fork 654
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
[eval] https tests #11638
base: development
Are you sure you want to change the base?
[eval] https tests #11638
Conversation
@Apprentice-Alchemist do you have an idea what we're doing wrong there that could cause this haxe/libs/mbedtls/mbedtls_stubs.c Lines 447 to 450 in db842bf
haxe/std/eval/_std/sys/ssl/Socket.hx Lines 27 to 37 in db842bf
Lines 444 to 453 in db842bf
Edit: |
Hmm. If I try As for that EBADF issue, the cause would be this function being called with a bad socket haxe/src/macro/eval/evalSsl.ml Lines 72 to 74 in db842bf
aka the issue would be somehwere here haxe/libs/mbedtls/mbedtls_stubs.c Lines 462 to 481 in db842bf
I don't know enough about Ocaml FFI to immediately see anything wrong with the bindings. |
Yes the file needs to be big enough to trigger that issue. The GC workaround makes me think we're allocating a bit too much there, and eval GC doesn't like while loops ? 🤔 |
I'm not very familiar with OCaml FFI either, but I always thought that only |
That |
Hmm, but it does have this |
Thinking about it some more, the real problem is probably that |
Yes I think that's what I'm trying to get at as well. |
Update please! |
51b28f6
to
6bda9cc
Compare
Well.. locally it's worse than before (the workaround isn't working anymore), even if CI seems fine so far 🤔 |
Works fine on my machine, with or without the workaround. |
You do enable https tests by commenting out this line, right? haxe/tests/unit/src/unit/TestHttps.hx Line 7 in 6bda9cc
I'm on Arch Linux too and I get this with or without the workaround:
(unless I use nightlies which are not compiled with mbedtls 3, then it's fine) |
Yes, I enabled https tests. Does the core dump give a useful backtrace? |
Doesn't give anything :/ Also, a similar fix might be needed for mac+hl for these tests to pass |
Regarding the HL+macOS failure, I don't think it's caused by GC issues or stuff like that. I did a little bit of debugging using CI (https://github.com/Apprentice-Alchemist/hashlink/actions/runs/8907771743/job/24462210850) Not yet sure what the root cause is, but I suspect it might be due to some compile option getting enable by Homebrew somehow? |
Ah, figured it out. Starting in MbedTLS 3.6 |
Ah nice, thanks! Would be nice to have a draft PR with your branch so that it could be done at some point (by you or someone else), and I could disable the test here in the meantime. As for my local issue, as long as it's only happening for me I guess it's ok... :/ |
I have merged #11655, please update the branch! |
I'm still getting
|
There's something wrong with mbedtls (or more likely how we use it), but I couldn't fix this yet.. :/
Also, hl on mac seems to have a similar issue 🤔