data61 / MP-SPDZ

Versatile framework for multi-party computation
Other
907 stars 279 forks source link

Memory corruption error when using ProtocolSet interface #615

Closed BeStrongok closed 2 years ago

BeStrongok commented 2 years ago

Dear authors, i'm using the low level interface to develop an mpc program. I have written the complete code, but when running the program i meet an error saying malloc(): memory corruption. I debug the code and find that the problem occurs when initializing the ProtocolSet class. There is the code:

template<class T>   // template T is Share type
void MPCProgram::run_step(vector<int64_t>& pubVec,
            weld::vec<int64_t>& dataVec, 
            bool reveal_shares,
            PyObject* op)
{
    auto& P = *this->P;
    ProtocolSetup<T> setup(P, params.prime_length);
    ProtocolSet<T> set(P, setup);   // This line occurs the memory corruption error
    auto& input = set.input;
    auto& output = set.output;
    input.reset_all(P);
}

The Player* P is the private member of class MPCProgram, can you help me? Thank you very much!

mkskeller commented 2 years ago

Can you submit a minimal working example? I cannot do anything about this without a way to reproduce the error.

mkskeller commented 2 years ago

One thing to note is that most of the code isn't thread-safe. In particular, you cannot use instances of Player in several threads, and you cannot create ProtocolSetup instances in several threads.

BeStrongok commented 2 years ago

One thing to note is that most of the code isn't thread-safe. In particular, you cannot use instances of Player in several threads, and you cannot create ProtocolSetup instances in several threads.

Thank you very much, i have solved this problem. The CMakeLists i wrote for my own project is wrong, i'm even surprised that the source code can be compiled. Now it can run normally. But i find that the runtime speed is slowly than addition-example.cpp, maybe there are two much parameter passing in these functions? I write several function to do protocol setup, secrete sharing and computation.

mkskeller commented 2 years ago

I can't comment on this without further information. Generally, doing more computation takes longer. The example computation is very simply, so I would expect almost any application to take more time.

BeStrongok commented 2 years ago

The computation is 3 party 10 thousands integer addtition with Shamir protocol, I find that the most time-consuming operation is reveal results form secret shares, which takes 3684ms. The code of this operation is:

    for (int i = 0; i<n; i++).  // n is 10000
    {
        output.init_open(P); 
        output.prepare_open(d[i]);    // d is the vector of results to be revealed
        output.exchange(P);
        bigint res = output.finalize_open();
        int64_t result = res.get_si();
        // cout << i << "'s result: " << result << endl;
    }
    // result check after opening
    set.check();
    auto t5 = std::chrono::high_resolution_clock::now();
    auto ms_int_4 = std::chrono::duration_cast<std::chrono::milliseconds> (t5 - t4);
    cout << "Time taken to run reveal_results function is : " << ms_int_4.count() << "ms" << endl;

But you say instances of Player cannot be used in multi-threads. Is there any way to optimize this time consumption through c++ code? I find that the for_range_parallel is very effective on reducing the runtime of the computation, but i don't figure out the principle of this api, it seems similar to dynamic loop unrolling, i see the compilation flag is set as -O3, does the compiler do loop unrolling automatically?

mkskeller commented 2 years ago

The idea of the C++ interface is that you call exchange() as little as possible. Your code calls it unnecessarily often, which leads to a large number of network rounds. The C++ will not recognize this. You should two different loops for prepare_open() and finalize_open().

mkskeller commented 2 years ago

See the structure of the inputs in Utils/paper-example.cpp for how to use this kind of interface.

mkskeller commented 2 years ago

Player instances can be used with multiple threads, but you need to use a separate instance per thread.

BeStrongok commented 2 years ago

Player instances can be used with multiple threads, but you need to use a separate instance per thread.

Thank you for your helpful reply!! I changed the code and found that the runtime is reduced significantly, thanks again! 👍