Why is Hash.buffer static?

[lvh]

https://github.com/abstractj/kalium/blob/master/src/main/java/org/abstractj/kalium/crypto/Hash.java#L28

I don't know Java very well anymore, and I don't know jnr-ffi at all, but I think this means that all Hash objects will share that buffer. Unless jnr-ffi somehow manages to copy this buffer synchronously and atomically for the C libsodium code that uses it, this sounds like a thread safety issue... What happens when fifty threads start trying to use this function at the same time?

Assuming this is a bug, some suggestions:

1. Don't make Hash.buffer static
2. Don't give Hash any instance attributes at all and manually do it in each method call

I have a minor preference for number 2.

[abstractj]

@lvh about that static field, that would be true if buffer wasn't being initialized at each method access like here and the remaining methods. Anyways, feel free to send a pull request with what do you have in mind.

About your question, probably fifty instances of a new byte array will be created. Buf if you have a snippet to test or reproduce a bug, please attach here.

[lvh]

I don't know if there's a bug there; I'd guess to see if there is I'd have to get a bunch of threads to try and use it at the same time :) (Of course, such an experiment can only show that there is a race condition, it can't show that there isn't...)

What would be the difference between:

public class Hash {
    private static byte[] buffer;
    public byte[] sha256(byte[] message) {
        buffer = new byte[SHA256BYTES];
        sodium().crypto_hash_sha256(buffer, message, message.length);
        return buffer;
    }
    // other methods...
}

and turning the buffer into a local:

public class Hash {
    public byte[] sha256(byte[] message) {
        byte[] buffer = new byte[SHA256BYTES];
        sodium().crypto_hash_sha256(buffer, message, message.length);
        return buffer;
    }
    // other methods...
}

... and, more importantly, what happens in this case:

public class Hash {
    private static byte[] buffer;
    public byte[] sha256(byte[] message) {
        buffer = new byte[SHA256BYTES];
        /* <------ THREAD 1 GETS SUSPENDED HERE */
        sodium().crypto_hash_sha256(buffer, message, message.length);
        return buffer;
    }
    // other methods...
}

... and then thread 2 enters the same method, sets a different buffer, calls crypto_hash_sha256 and completes. At some point thread 1 wakes up again; which buffer does it see?

I will send a pull request with the example above, but if you'd like I can also try and see if I can get the race condition to work?

[abstractj]

@lvh tbh I didn't get the chance to debug and see what JVM says, was more convenience then design decision.

Also, I'm not against on making local, just send a pull request, if the tests are passing I'm fine on it. If you are willing to do more tests or check the race condition, is also very appreciated.

Thanks @lvh you rock.