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

Occasional NullPointerException in BoltRequest#executeRequest on start-up #595

Closed
steffen-harbich-cognitum opened this issue Jan 22, 2019 · 2 comments
Assignees
Milestone

Comments

@steffen-harbich-cognitum

Expected Behavior

Expecting to always start without null pointer.

Current Behavior

Sometimes (occasional) when starting my Spring Boot 2.1.1 application (included is OGM 3.1.5, bolt driver), a NullPointerException occurs with stack trace:

Caused by: java.lang.NullPointerException: null
at org.neo4j.ogm.drivers.bolt.request.BoltRequest.executeRequest(BoltRequest.java:149) ~[neo4j-ogm-bolt-driver-3.1.5.jar:3.1.5]
at org.neo4j.ogm.drivers.bolt.request.BoltRequest.execute(BoltRequest.java:143) ~[neo4j-ogm-bolt-driver-3.1.5.jar:3.1.5]
at org.neo4j.ogm.session.delegates.ExecuteQueriesDelegate.lambda$query$0(ExecuteQueriesDelegate.java:102) ~[neo4j-ogm-core-3.1.5.jar:3.1.5]
at org.neo4j.ogm.session.Neo4jSession.doInTransaction(Neo4jSession.java:569) ~[neo4j-ogm-core-3.1.5.jar:3.1.5]
at org.neo4j.ogm.session.Neo4jSession.doInTransaction(Neo4jSession.java:553) ~[neo4j-ogm-core-3.1.5.jar:3.1.5]
at org.neo4j.ogm.session.delegates.ExecuteQueriesDelegate.query(ExecuteQueriesDelegate.java:100) ~[neo4j-ogm-core-3.1.5.jar:3.1.5]
at org.neo4j.ogm.session.delegates.ExecuteQueriesDelegate.query(ExecuteQueriesDelegate.java:80) ~[neo4j-ogm-core-3.1.5.jar:3.1.5]
at org.neo4j.ogm.session.Neo4jSession.query(Neo4jSession.java:413) ~[neo4j-ogm-core-3.1.5.jar:3.1.5]
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:na]
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
at java.base/java.lang.reflect.Method.invoke(Method.java:566) ~[na:na]
at org.springframework.data.neo4j.transaction.SharedSessionCreator$SharedSessionInvocationHandler.invoke(SharedSessionCreator.java:122) ~[spring-data-neo4j-5.1.3.RELEASE.jar:5.1.3.RELEASE]
at com.sun.proxy.$Proxy146.query(Unknown Source) ~[na:na]

It happens unpredictable which is already a pointer to a multi-thread issue.

Possible Solution

I debugged into it and "cypherModification" is null when the NPE is thrown. The problematic code seems to be the lazy initialization:

  private volatile Function<String, String> cypherModification;
   @Override
   public final Function<String, String> getCypherModification() {

       Function<String, String> loadedCypherModification = this.cypherModification;
       if(loadedCypherModification == null) {
           synchronized (this) {
               if(this.cypherModification == null) {
                   loadedCypherModification = this.cypherModification = loadCypherModifications();
               }
           }
       }
       return loadedCypherModification;
   }

Looks like a double-checked locking (https://en.wikipedia.org/wiki/Double-checked_locking) but isn't correctly done. Assume that 2 threads arrived at the synchronized block at the same time. One of them wins, loads the cypher modifications, sets it to the local variable and returns it. The 2nd thread, however, enters then the synchronized block and detects that "cypherModification" is not null and hence, does not do anything there. The local variable "loadedCypherModification" is still null and will be returned.

In Wikipedia, there is an example:

// Works with acquire/release semantics for volatile in Java 1.5 and later
// Broken under Java 1.4 and earlier semantics for volatile
class Foo {
    private volatile Helper helper;
    public Helper getHelper() {
        Helper localRef = helper;
        if (localRef == null) {
            synchronized (this) {
                localRef = helper;  // <------ missing in ogm code
                if (localRef == null) {
                    helper = localRef = new Helper();
                }
            }
        }
        return localRef;
    }

    // other functions and members...
}

I think a loadedCypherModification = this.cypherModification as first line of the synchronized block would be sufficient to fix the problem.

Steps to Reproduce (for bugs)

Due to its occasional occurence, it is hard to reproduce.

Context

Your Environment

  • OGM Version used: 3.1.5
  • Java Version used: 11
  • Neo4J Version used: 3.5.0
  • Bolt Driver Version used (if applicable): 3.1.5
  • Operating System and Version: Windows 10
  • Link to your project:
@michael-simons michael-simons self-assigned this Jan 23, 2019
@michael-simons
Copy link
Collaborator

🤦‍♂️ You're right. Will be fixed soon.

michael-simons added a commit that referenced this issue Jan 23, 2019
This closes #595.

# Conflicts:
#	api/src/main/java/org/neo4j/ogm/driver/AbstractConfigurableDriver.java
@michael-simons
Copy link
Collaborator

Fixed in Master and 3.1.x.

Thanks a lot for reporting it.

@michael-simons michael-simons added this to the 3.1.7 milestone Jan 23, 2019
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

2 participants