ethereum / ethereumj

DEPRECATED! Java implementation of the Ethereum yellowpaper. For JSON-RPC and other client features check Ethereum Harmony
GNU Lesser General Public License v3.0
2.18k stars 1.09k forks source link

DBSettings should be initialized in write lock. #1255

Closed chenjw13097 closed 5 years ago

chenjw13097 commented 5 years ago

Current code is:

    public void init(DbSettings settings) {
        (1) this.settings = settings;
        (2) resetDbLock.writeLock().lock();
        try {
            logger.debug("~> RocksDbDataSource.init(): " + name);

If two threads (named T1 and T2) call the method with different DbSettings, follow code may go wrong::

    options.setMaxOpenFiles(settings.getMaxOpenFiles());
    options.setIncreaseParallelism(settings.getMaxThreads());

For example, T1(1)->T2(1)->T1(2)->T2(2)),T1 will create the DB with settings passed by T2. For now, it still works well. But it may go wrong if init is called outside in future code.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.1%) to 56.914% when pulling d1f4eb6fe467a36d398e52771676f612d9ceb85f on chenjw13097:develop into 73acba3005aba45cc9f00bbb6e58db8b1de9db8a on ethereum:develop.

zilm13 commented 5 years ago

@chenjw13097 if you don't want 2 threads to set different settings, you should either synchronize method or init settings only once, in constructor, I don't understand how could you prevent it with moving this line to try block