Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NullPointerException on concurrent requests via async CoapClient#advanced #58

Closed
eugene-nikolaev opened this issue Jun 15, 2016 · 0 comments

Comments

@eugene-nikolaev
Copy link
Contributor

eugene-nikolaev commented Jun 15, 2016

Time-to-time my tests were failing with the following stacktrace:

32 INFO [CoapEndpoint]: Starting endpoint at 0.0.0.0/0.0.0.0:0 - (org.eclipse.californium.core.network.CoapEndpoint.java:115) start() in thread Thread-6 at (2016-06-15 19:46:40)
    at org.eclipse.californium.core.network.CoapEndpoint.runInProtocolStage(CoapEndpoint.java:692)
    at org.eclipse.californium.core.network.CoapEndpoint.sendRequest(CoapEndpoint.java:390)
    at org.eclipse.californium.core.CoapClient.send(CoapClient.java:897)
    at org.eclipse.californium.core.CoapClient.send(CoapClient.java:879)
    at org.eclipse.californium.core.CoapClient.asynchronous(CoapClient.java:742)
    at org.eclipse.californium.core.CoapClient.advanced(CoapClient.java:663)
    at <SKIPPED>
    at java.lang.Thread.run(Thread.java:745)
32 INFO [EndpointManager]: Created implicit default endpoint 0.0.0.0/0.0.0.0:39102 - (org.eclipse.californium.core.network.EndpointManager.java:98) createDefaultEndpoint() in thread Thread-6 at (2016-06-15 19:46:40)

The code made concurrent asynchronous requests via CoapClient using default endpoint (no endpoint was specified).

After some debug, I discovered a race condition.

EndpointManager:

   public Endpoint getDefaultEndpoint() {
        if (default_endpoint == null) {
            createDefaultEndpoint();
        }
        return default_endpoint;
    }

    private synchronized void createDefaultEndpoint() {
        if (default_endpoint != null) return;

        default_endpoint = new CoapEndpoint();

        try {
            default_endpoint.start();
                        ...

CoapEndpoint:

    public synchronized void start() throws IOException {
        ....

        if (this.executor == null) {
                             ...
            setExecutor(Executors.newSingleThreadScheduledExecutor(
                    new Utils.DaemonThreadFactory("CoapEndpoint-" + connector.getAddress() + '#'))); //$NON-NLS-1$
                 ...

     private void runInProtocolStage(final Runnable task) {
        executor.execute(new Runnable() {
                            ...

So, when two threads invoked CoapClient#advanced(CoapHandler, Request) they called EndpointManager#getDefaultEndpoint somewhere down in the hierarchy.

getDefaultEndpoint invoked synchronized createDefaultEndpoint but as long as getDefaultEndpoint is not synchronized, second thread got default_endpoint != null and proceeded. But the default_endpoint was not started yet so CoapEndpoint#executor was not filled.

It is actually a cause of that NPE.
createDefaultSecureEndpoint is not synchronized too.
I am preparing a PR (actually just adding two 'synchronized') now.
But I am not sure it is possible to make a unit test for this case.

sophokles73 pushed a commit that referenced this issue Jun 25, 2016
…ent#advanced

Signed-off-by: Eugene Nikolaev <eug.nikolaev@gmail.com>
sophokles73 pushed a commit that referenced this issue Jun 25, 2016
…ent#advanced

Signed-off-by: Eugene Nikolaev <eug.nikolaev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant