java-conflux-sdk icon indicating copy to clipboard operation
java-conflux-sdk copied to clipboard

It looks like sendAndGet() is not thread-safe

Open linyufly opened this issue 3 years ago • 1 comments

Hi folks,

I tried to use two threads to make different contract calls. Sometimes their responses may interfere with each other.

For this test, I defined the following class.

    private class ContractCallThread extends Thread {
        public ContractCallThread(ContractCall contract, String method, Type<?>... args) {
            this.contract = contract;
            this.method = method;
            this.args = args;
        }

        public void run() {
            this.result = this.contract.call(this.method, this.args).sendAndGet();
        }

        public String result() {
            return this.result;
        }

        private ContractCall contract = null;
        private String method = null;
        private Type<?>[] args = null;
        private String result = null;
    }

The test code is as follows.

      while (true) {
            ContractCallThread getOutputsThread1 = new ContractCallThread(<contract_name>, <method_name>, <args_1>);
            ContractCallThread getOutputsThread2 = new ContractCallThread(<contract_name>, <method_name>, <args_2>);
            getOutputsThread1.start();
            getOutputsThread2.start();
            getOutputsThread1.join();
            String result1 = getOutputsThread1.result();
            <check_result1>
            getOutputsThread2.join();
      }

You'll soon get a result1 corresponding to args_2.

Could you please take a look? Thanks!

-linyufly

linyufly avatar Jun 01 '21 00:06 linyufly

Update:

Further experiments showed that the thread-safety risk lies in ContractCall.

The following code avoids the race condition.

            ContractCall contract1 = new ContractCall(cfx, <contract_address>);
            ContractCall contract2 = new ContractCall(cfx, <contract_address>);
            ContractCallThread getOutputsThread1 = new ContractCallThread(contract1, <method_name>, <args_1>);
            ContractCallThread getOutputsThread2 = new ContractCallThread(contract2, <method_name>, <args_2>);

contract1 and contract2 refer to the same contract, but they are different objects.

The following code still has the race condition.

            ContractCall contract1 = new ContractCall(cfx, <contract_address>);
            ContractCall contract2 = contract1;
            ContractCallThread getOutputsThread1 = new ContractCallThread(contract1, <method_name>, <args_1>);
            ContractCallThread getOutputsThread2 = new ContractCallThread(contract2, <method_name>, <args_2>);

-linyufly

linyufly avatar Jun 01 '21 04:06 linyufly