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.

Sign up for GitHub

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

lightclient: Fix wasm socket closure called after being dropped #1289

Merged
merged 10 commits into from
Nov 29, 2023
Merged

lightclient: Fix wasm socket closure called after being dropped #1289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f3eb4cd
lightclient: Close wasm socket while dropping from connecting state
lexnv Nov 23, 2023
cfac283
lightclient: Construct one time only closures
lexnv Nov 24, 2023
860f0e0
testing: Enable console logs for lightclient WASM testing
lexnv Nov 24, 2023
8660940
lightclient: Separate wakes and check connectivity on poll_read
lexnv Nov 24, 2023
b2e6d41
lightclient: Close the socket depending on internal state
lexnv Nov 24, 2023
78f4d88
Revert "lightclient: Separate wakes and check connectivity on poll_read"
lexnv Nov 24, 2023
1cdee8f
lightclient: Return pending if socket is opening from poll_read
lexnv Nov 24, 2023
bd13879
lightclient: Close the socket on `poll_close`
lexnv Nov 24, 2023
099f6ee
lightclient: Reset closures on Drop to avoid recursive invokation
lexnv Nov 27, 2023
abe77b5
lightclient: Close the socket if not already closing
lexnv Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 24 additions & 7 deletions lightclient/src/platform/wasm_socket.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl WasmSocket {

let error_callback = Closure::<dyn FnMut(_)>::new({
let inner = inner.clone();
move |_| {
move |_event: web_sys::Event| {
// Callback does not provide useful information, signal it back to the stream.
let mut inner = inner.lock().expect("Mutex is poised; qed");
inner.state = ConnectionState::Error;
Expand All @@ -136,7 +136,7 @@ impl WasmSocket {

let close_callback = Closure::<dyn FnMut(_)>::new({
let inner = inner.clone();
move |_| {
move |_event: web_sys::CloseEvent| {
let mut inner = inner.lock().expect("Mutex is poised; qed");
inner.state = ConnectionState::Closed;

Expand Down Expand Up @@ -171,6 +171,10 @@ impl AsyncRead for WasmSocket {
let mut inner = self.inner.lock().expect("Mutex is poised; qed");
inner.waker = Some(cx.waker().clone());

if self.socket.ready_state() == web_sys::WebSocket::CONNECTING {
return Poll::Pending;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think this is ook because any of the callbacks will wake the waker when JS clals them, ensuring we call this poll_read again.

}

match inner.state {
ConnectionState::Error => {
Poll::Ready(Err(io::Error::new(io::ErrorKind::Other, "Socket error")))
Expand Down Expand Up @@ -221,17 +225,30 @@ impl AsyncWrite for WasmSocket {
Poll::Ready(Ok(()))
}

fn poll_close(self: Pin<&mut Self>, _: &mut Context<'_>) -> Poll<Result<(), io::Error>> {
Poll::Ready(Ok(()))
fn poll_close(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<(), io::Error>> {
if self.socket.ready_state() == web_sys::WebSocket::CLOSED {
return Poll::Ready(Ok(()));
}

if self.socket.ready_state() != web_sys::WebSocket::CLOSING {
let _ = self.socket.close();
}

let mut inner = self.inner.lock().expect("Mutex is poised; qed");
inner.waker = Some(cx.waker().clone());
Poll::Pending
}
}

impl Drop for WasmSocket {
fn drop(&mut self) {
let inner = self.inner.lock().expect("Mutex is poised; qed");

if inner.state == ConnectionState::Opened {
if self.socket.ready_state() != web_sys::WebSocket::CLOSING {
let _ = self.socket.close();
}

self.socket.set_onopen(None);
self.socket.set_onmessage(None);
self.socket.set_onerror(None);
self.socket.set_onclose(None);
}
}
2 changes: 2 additions & 0 deletions testing/wasm-lightclient-tests/tests/wasm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser);

#[wasm_bindgen_test]
async fn light_client_works() {
console_error_panic_hook::set_once();

let api: LightClient<PolkadotConfig> = LightClientBuilder::new()
.build_from_url("wss://rpc.polkadot.io:443")
.await
Expand Down