Skip to content

Conversation

RCasatta
Copy link
Collaborator

@RCasatta RCasatta commented Sep 8, 2025

No description provided.

@shesek
Copy link
Collaborator

shesek commented Sep 25, 2025

The fix looks good 👍

However I'm unable to get the new test case to fail with the original code. The expect("send failed") is reached within the acceptor thread, but it doesn't fail the main rpc server thread and doesn't fail the test.

I can get the test to fail if I make the main rpc thread explicitly close the acceptor channel and wait for the acceptor thread to exit (implemented in shesek@d0eabd4) -- which seems like a good idea regardless. (the test does pass with your fix applied.)

This fix isn't right -- it works for the test case, but if no new connections are made then the TcpListener::accept() call will just hang indefinitely and never reach the point of trying to use the acceptor channel and closing its thread, which would also hang the main rpc thread.

@shesek
Copy link
Collaborator

shesek commented Sep 25, 2025

It seems the CI tests are failing for an unrelated reason that we already ran into before, due to running multiple parallel electrs instances within the same process. Something is getting mixed up with the RPC thread pool, one electrs instance ends up using sockets created for the other and gets unexpected responses that result in a panic.

The easy solution is to #[ignore] by default and run the test singularly separately, like we did with test_electrum_raw. But of course the more ideal one is to fix the bug. :)

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