parity-clib: `async C bindings to RPC requests` + `subscribe/unsubscribe to websocket events` #9920

niklasad1 · 2018-11-14T13:04:17Z

Attempt to close #9271

This PR changes to following in parity-clib (C and Java bindings):

asynchronous RPC queries (allocates a background thread that runs for at most X seconds)
subscribe to WebSocket events (runs in a background thread until it is cancled)
unsubscribe from WebSocket events

Additional changes:

modify party-cpp-example
add Java example application

TODOs:

Update Java bindings (Parity.java) and test it with a Java application
Change callback structure

Pitfalls/suggested improvements, should be fixed in this PR?!

Dropping a GlobalRef in a detached thread (JNI related https://docs.rs/jni/0.10.2/jni/objects/struct.GlobalRef.html)
Use a shared thread for WebSocket subscriptions (really hard without major refactoring because the parity-clib doesn't keep state)
Use of deprecated finalize() in Parity.Java (I have looked into it very briefly and the way to go is to use PhantomReference instead but requires more boilerplate code and I like currently how simple the Parity class is)

logger/src/lib.rs

parity-clib/src/lib.rs

niklasad1 · 2018-11-17T00:38:22Z

The PR should be in reviewable stable state now seem to work both for the light and full client.

The easiest way if you want to test it, grab the update cpp example and then make sure that you are fully-synced otherwise ChainNotify doesn't notify the PubSub Client

parity-clib-examples/cpp/main.cpp

parity-clib/parity.h

niklasad1 · 2018-11-21T17:06:48Z

@tomaka

I have started to look at the Java bindings and the synchronous part works fine. However, the asynchronous part is a different story in order to invoke a java method I have to do by using the JNI/JVM environment AFAIU

Something like

    env.call_method(
        callback, 
        "callback", 
        "(JLjava/lang/Object;Ljava/lang/Object;J)V", 
        &[JValue::Long(0), JValue::Object(null), JValue::Object(null), JValue::Long(0)]
  ).expect("Java method call failed");

Thus, using the C API "out-of-the-box" won't work. So as I see it I can either:

Introduce CCallback and JCallback in the the C-API
Re-implement the functionality in the java module

Thoughts?

niklasad1 · 2018-11-22T11:25:16Z

parity-clib/src/lib.rs

+impl CallbackStr {
+	fn call(&self, msg: &[u8]) {
+		if let Some(ref cb) = self.f {
+			let cstr = CString::new(msg).expect("valid string with no null bytes in the middle; qed").into_raw();


This may be a performance bottleneck, it is O(n) + additional allocation but nice to have from a C/C++ perspective but for other languages such as Java, I expect that it still requires to copy these raw bytes into respective String type!

parity-clib-examples/java/Main.java

* Subscribing to websockets for the full-client works

* Enforce C+11

* Use it the example to avoid using global variables

Forgot to pass the JNIEnv environment since it is an instance method

* Remove Java dependency by constructing a valid Java String in the callback

* `cpp` example pass in a struct instead to determines `callback kind` * `java` add a instance variable the class `Callback` to determine `callback kind`

Co-Authored-By: niklasad1 <[email protected]>

niklasad1 · 2019-01-02T13:49:29Z

@5chdn The parity-clib/example was moved so the source path didn't work. I will fix this soon

* fix(remove needless unsafe blocks) * style(nits) * fix(parity-clib): eliminate repetitive event loops * revert(java bindings): safe rust -> unsafe rust These functions can still end up with `UB` thus should be unsafe * fix(grumbles): make Callback trait `pub (crate)`

* master: no volumes are needed, just run -v volume:/path/in/the/container (#10345) Fixed misstype (#10351) snap: prefix version and populate candidate channel (#10343) Bundle protocol and packet_id together in chain sync (#10315) role back docker build image and docker deploy image to ubuntu:xenial based (#10338) change docker image based on debian instead of ubuntu due to the chan… (#10336) Don't add discovery initiators to the node table (#10305) fix(docker): fix not receives SIGINT (#10059) snap: official image / test (#10168) fix(add helper for timestamp overflows) (#10330) Additional error for invalid gas (#10327) Revive parity_setMinGasPrice RPC call (#10294) Add Statetest support for Constantinople Fix (#10323) fix(parity-clib): grumbles that were not addressed in #9920 (#10154) fix(light-rpc): Make `light_sync` generic (#10238) fix publish job (#10317) Secure WS-RPC: grant access to all apis (#10246) Make specification of protocol in SyncRequester::send_request explicit (#10295)

niklasad1 requested review from tomaka and tomusdrw November 14, 2018 13:04

niklasad1 commented Nov 14, 2018

View reviewed changes

logger/src/lib.rs Outdated Show resolved Hide resolved

niklasad1 force-pushed the parity-clib/jsonrpc-callback branch from 3bae6fb to 86ff729 Compare November 14, 2018 13:09

niklasad1 added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Nov 14, 2018

tomaka reviewed Nov 14, 2018

View reviewed changes

parity-clib/src/lib.rs Outdated Show resolved Hide resolved

niklasad1 changed the title ~~parity-clib: async C bindings to RPC queries~~ [WIP] parity-clib: async C bindings to RPC queries Nov 14, 2018

niklasad1 added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 14, 2018

niklasad1 force-pushed the parity-clib/jsonrpc-callback branch 4 times, most recently from 5f7f1ae to 350b13a Compare November 17, 2018 00:30

niklasad1 changed the title ~~[WIP] parity-clib: async C bindings to RPC queries~~ parity-clib: async C bindings to RPC query + subscribe/unsubscribe to websocket events Nov 17, 2018

niklasad1 changed the title ~~parity-clib: async C bindings to RPC query + subscribe/unsubscribe to websocket events~~ parity-clib: async C bindings to RPC requests + subscribe/unsubscribe to websocket events Nov 17, 2018

niklasad1 added A0-pleasereview 🤓 Pull request needs code review. M6-rpcapi 📣 RPC API. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Nov 17, 2018

niklasad1 force-pushed the parity-clib/jsonrpc-callback branch 3 times, most recently from 3e4c677 to c1a8cc9 Compare November 18, 2018 23:21

tomaka suggested changes Nov 19, 2018

View reviewed changes

niklasad1 force-pushed the parity-clib/jsonrpc-callback branch 2 times, most recently from b897ac4 to 55a0d2e Compare November 20, 2018 12:41

niklasad1 commented Nov 22, 2018

View reviewed changes

niklasad1 force-pushed the parity-clib/jsonrpc-callback branch from 98b3f18 to 8968c16 Compare November 23, 2018 16:57

niklasad1 commented Nov 23, 2018

View reviewed changes

parity-clib-examples/java/Main.java Outdated Show resolved Hide resolved

niklasad1 and others added 21 commits January 2, 2019 14:02

feat(seperate bindings for ws and rpc)

6cd0c13

* Subscribing to websockets for the full-client works

feat(c binding unsubscribe_from_websocket)

78664a0

fix(tests): tweak CMake build config

bf450a3

* Enforce C+11

refactor(parity-cpp-example) : cpp:ify

1cca514

fix(typedefs) : revert typedefs parity-clib

79c0576

docs(nits)

ac15960

fix(simplify websocket_unsubscribe)

9c2d49e

refactor(cpp example) : more subscriptions

1f93ffa

fix(callback type) : address grumbles on callback

ee38cf1

* Use it the example to avoid using global variables

docs(nits) - don't mention arc

f67523e

fix(jni bindings): fix compile errors

22f9e9f

feat(java example and updated java bindings)

f35d882

fix(java example) : run both full and light client

489fc7a

fix(Java shutdown) : unsubscribe to sessions

0a4aff1

Forgot to pass the JNIEnv environment since it is an instance method

feat(return valid JString)

e457835

* Remove Java dependency by constructing a valid Java String in the callback

fix(logger) : remove rpc trace log

8d73aa2

fix(format)

fc3bf56

fix(parity-clib): remove needless callback type

269f8e8

fix(parity-clib-examples) : update examples

019ab72

* `cpp` example pass in a struct instead to determines `callback kind` * `java` add a instance variable the class `Callback` to determine `callback kind`

fix(review comments): docs and format

981d320

Update parity-clib/src/java.rs

6014461

Co-Authored-By: niklasad1 <[email protected]>

niklasad1 force-pushed the parity-clib/jsonrpc-callback branch from e4be3e4 to 48d5706 Compare January 2, 2019 13:11

fix(bad merge + spelling)

a50b062

niklasad1 force-pushed the parity-clib/jsonrpc-callback branch from 48d5706 to a50b062 Compare January 2, 2019 13:13

fix(move examples to parity-clib/examples)

9164b32

5chdn merged commit b4f8bba into master Jan 2, 2019

5chdn deleted the parity-clib/jsonrpc-callback branch January 2, 2019 15:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

parity-clib: `async C bindings to RPC requests` + `subscribe/unsubscribe to websocket events` #9920

parity-clib: `async C bindings to RPC requests` + `subscribe/unsubscribe to websocket events` #9920

niklasad1 commented Nov 14, 2018 •

edited

Loading

niklasad1 commented Nov 17, 2018

niklasad1 commented Nov 21, 2018

niklasad1 Nov 22, 2018 •

edited

Loading

niklasad1 commented Jan 2, 2019

parity-clib: async C bindings to RPC requests + subscribe/unsubscribe to websocket events #9920

parity-clib: async C bindings to RPC requests + subscribe/unsubscribe to websocket events #9920

Conversation

niklasad1 commented Nov 14, 2018 • edited Loading

niklasad1 commented Nov 17, 2018

niklasad1 commented Nov 21, 2018

niklasad1 Nov 22, 2018 • edited Loading

Choose a reason for hiding this comment

niklasad1 commented Jan 2, 2019

parity-clib: `async C bindings to RPC requests` + `subscribe/unsubscribe to websocket events` #9920

parity-clib: `async C bindings to RPC requests` + `subscribe/unsubscribe to websocket events` #9920

niklasad1 commented Nov 14, 2018 •

edited

Loading

niklasad1 Nov 22, 2018 •

edited

Loading