Null access is always fun



  • I just found this code in some legacy crap I am trying to replace.  I am not sure what this person was thinking, but they obviously didn't test it.


        protected static ArrayList getIExecutableObjects(Connection con, IExecutable criteria)
        {
            if (criteria == null)
                return criteria.getDataBeanContainer();

            return executeSQL(criteria, con, criteria.getSelect());
        }



  • Probably he wanted to make sure a null exception gets thrown when criteria is null, because passing null to executeSQL does something really, really bad.


    Of course, in that case he could have just thrown an exception manually.


    (Making the buttumption here that trying to access null will raise an exception. Or am I wrong?)



  • @derula said:

    Probably he wanted to make sure a null exception gets thrown when criteria is null, because passing null to executeSQL does something really, really bad.
     

    Not an excuse. You can just throw your own. Also it will happen with the method call, because it contains 'criteria.getSelect()'



  • I'm not sure about Java, but in C++ this function could start with

    if (!this) return defaultValue;


    Not that it would be a good style...



  • I'm struggling to work out what was intended... My first though was that there should've been brackets with the if, but the first line would be a return anyway...

    My second thought was that executeSQL might be really messed up and do something with a null that we want, but firstly it'd be much tidier to just pass it a null if that's the case and secondly it'd crash on the getSelect...

    What was was the actual desired functionality? Just to throw an exception if it's passed a null or? (Just to try to get a glimpse into the mind of the original coder :P)



  • @bugmenot1 said:

    I'm not sure about Java, but in C++ this function could start with

    if (!this) return defaultValue;




    Not that it would be a good style...

    I always start my java methods with:
    if (this == null) throw new NullPointerException();
    
    Catches many errors.


  • @bugmenot1 said:

    I'm not sure about Java, but in C++ this function could start with

    if (!this) return defaultValue;




    Not that it would be a good style...

    I always start my java methods with:
    if (this == null) throw new NullPointerException();
    
    Catches many errors.

  • :belt_onion:

    @derula said:

    <snip>

    (Making the buttumption here that trying to access null will raise an exception. Or am I wrong?)

    Except if "getDataBeanContainer"is a static function which doesn't have a "this" pointer/reference. (Too lazy to try it out but it feels right)



  • @derula said:

    Probably he wanted to make sure a null exception gets thrown when criteria is null, because passing null to executeSQL does something really, really bad.
    Not applicable:

    return executeSQL(criteria, con, criteria.getSelect());



  • @Zecc said:

    @derula said:

    Probably he wanted to make sure a null exception gets thrown when criteria is null, because passing null to executeSQL does something really, really bad.
    Not applicable:

    return executeSQL(criteria, con, criteria.getSelect());


    Unless getSelect() is static. Ha



  • @ounos said:

    I always start my java methods with:
    if (this == null) throw new NullPointerException();
    
    Catches many errors.
    Never ever have I stumbled upon a this being null in Java. Can you elaborate a bit about the situations where that may happen?


  • @TheRider said:

    Never ever have I stumbled upon a this being null in Java. Can you elaborate a bit about the situations where that may happen?
    Yes, the way I see it either you are on a successfully called instance method (in which case 'this' isn't null) or on a static method (in which case 'this' isn't define.

    Are we missing something here?



  • @Zecc said:

    (in which case 'this' isn't define.

    Are we missing something here?

    ...besides the 'd' in "defined" and the closing parenthesis?



  •  I've seen this kind of thing in C++, where x is null, but people call x->func(). It succeeds as long as func() is static or doesn't refer to any class-local variables (as 'this' would be null also). 



  • @TheRider said:

    @ounos said:

    I always start my java methods with:

    if (this == null) throw new NullPointerException();
    

    Catches many errors.

    Never ever have I stumbled upon a this being null in Java. Can you elaborate a bit about the situations where that may happen?

    It is very rare but painful to track. Sometimes you create an object in one thread and use it in another. Without proper locking "this" could be uninitialized. But you need to do this in every instance method of shared objects.



  • @fyjham said:

    I'm struggling to work out what was intended... My first though was that there should've been brackets with the if, but the first line would be a return anyway...

     

    It's Java, you don't need brackets on an if statement with a single line of execution after it.  

    @fyjham said:

    My second thought was that executeSQL might be really messed up and do something with a null that we want, but firstly it'd be much tidier to just pass it a null if that's the case and secondly it'd crash on the getSelect...

    There is nothing I can think of that you would "want" to do with null in executing a SQL statement.

    @fyjham said:

    What was was the actual desired functionality? Just to throw an exception if it's passed a null or? (Just to try to get a glimpse into the mind of the original coder :P)

    Desired functionality is to get a Collection of elements from the database that hold data specific to a trade ticket.

    Here are the comments... so your guess is as good as mine.

     

        /**
         *     Gets a collection of IDataBean objects.
         *
         * @param con The connection to use in this query
         * @param criteria The search criteria is contained in this object
         * @return a collection of IDataBean objects
         */



  • @ounos said:

    @TheRider said:

    @ounos said:

    I always start my java methods with:
    if (this == null) throw new NullPointerException();
    

    Catches many errors.

    Never ever have I stumbled upon a this being null in Java. Can you elaborate a bit about the situations where that may happen?
    It is very rare but painful to track. Sometimes you create an object in one thread and use it in another. Without proper locking "this" could be uninitialized. But you need to do this in every instance method of shared objects.
     

    I call bullshit. "This" would not be 'uninitialized'.  If you are inside of a method there is a reference to 'this'.  Look at it like the stupid house/blueprint Object thingy taught to first year programming students.  You can't be inside of a room without there being a house.  Fucking impossible, unless you live in some 5th dimension.  Now, with threads involved, you may be in the wrong house, but you're still in a house.



  • @amischiefr said:

    @ounos said:

    @TheRider said:

    @ounos said:

    I always start my java methods with:

    if (this == null) throw new NullPointerException();
    

    Catches many errors.

    Never ever have I stumbled upon a this being null in Java. Can you elaborate a bit about the situations where that may happen?

    It is very rare but painful to track. Sometimes you create an object in one thread and use it in another. Without proper locking "this" could be uninitialized. But you need to do this in every instance method of shared objects.
     

    I call bullshit. "This" would not be 'uninitialized'.  If you are inside of a method there is a reference to 'this'.  Look at it like the stupid house/blueprint Object thingy taught to first year programming students.  You can't be inside of a room without there being a house.  Fucking impossible, unless you live in some 5th dimension.  Now, with threads involved, you may be in the wrong house, but you're still in a house.


    True, if you only code in a single processor. "This" is just a register and its initialization can be reordered as a typical cpu optimization.



  • @ounos said:

    True, if you only code in a single processor. "This" is just a register and its initialization can be reordered as a typical cpu optimization.

     

    I think that I just thought of the answer.  Err.. nevermind. 

    I can see the 'this' reference being null when using Java Reflection.  I will concede this. But multithreaded has nothing to do with it. 



  • @amischiefr said:

    @ounos said:


    True, if you only code in a single processor. "This" is just a register and its initialization can be reordered as a typical cpu optimization.

     

    I think that I just thought of the answer.  Err.. nevermind. 

    I can see the 'this' reference being null when using Java Reflection.  I will concede this. But multithreaded has nothing to do with it. 


    You might also want to read why the double checked locking idiom was broken (allowing fields to be seen uninitialized), to realize that multithreading has to do with it.



  • @amischiefr said:

    @ounos said:

    @TheRider said:

    @ounos said:

    I always start my java methods with:
    if (this == null) throw new NullPointerException();
    

    Catches many errors.

    Never ever have I stumbled upon a this being null in Java. Can you elaborate a bit about the situations where that may happen?
    It is very rare but painful to track. Sometimes you create an object in one thread and use it in another. Without proper locking "this" could be uninitialized. But you need to do this in every instance method of shared objects.
     

    I call bullshit. "This" would not be 'uninitialized'.  If you are inside of a method there is a reference to 'this'.  Look at it like the stupid house/blueprint Object thingy taught to first year programming students.  You can't be inside of a room without there being a house.  Fucking impossible, unless you live in some 5th dimension.  Now, with threads involved, you may be in the wrong house, but you're still in a house.

    Obvious troll is obvious.


  • @MurfWTF said:

    Obvious troll is obvious.
     

    Great contribution to the post numbnutz.  ounos is the only person in this thread, if you read all of the posts, that believes the 'this' reference in Java can be null.  So, care to explain how this is trolling kid?  

    If you don't have something to contribute, or at least a funny comment, please stfu.



  • @amischiefr said:

    @MurfWTF said:

    Obvious troll is obvious.
     

    Great contribution to the post numbnutz.  ounos is the only person in this thread, if you read all of the posts, that believes the 'this' reference in Java can be null.  So, care to explain how this is trolling kid?  

    If you don't have something to contribute, or at least a funny comment, please stfu.


    To be honest, I also had the impression that you were trolling.



  • @amischiefr said:

    @MurfWTF said:

    Obvious troll is obvious.
     

    Great contribution to the post numbnutz.  ounos is the only person in this thread, if you read all of the posts, that believes the 'this' reference in Java can be null.  So, care to explain how this is trolling kid?  

    If you don't have something to contribute, or at least a funny comment, please stfu.

     

    Not you, numbnutz.



  • @amischiefr said:

    It's Java, you don't need brackets on an if statement with a single line of execution after it.

    Yeah, I get that. I mean as in both methods were intended when it wasn't null. But since the first returns that wasn't feasible ;) Sorta like this:

    if (criteria == null)
    {
       return criteria.getDataBeanContainer();
       return executeSQL(criteria, con, criteria.getSelect());
    }

    @amischiefr said:

    There is nothing I can think of that you would "want" to do with null in executing a SQL statement.

    Indeed, hence the consideration that executeSQL may be trwtf (Unless I'm mistaken and it's part of java, but I'm pretty sure in java you'd have to at least write down the object it's in to call it if it wasn't part of the current object). There's obviously nothing "correct" about the code, I was just trying to find the actual point of mistake (Normally you can bring it all back to 1 thing, it's a classy kind of WTF when you can't even see what the person was trying to do :P



  • @amischiefr said:

    @ounos said:

    True, if you only code in a single processor. "This" is just a register and its initialization can be reordered as a typical cpu optimization.

     

    I think that I just thought of the answer.  Err.. nevermind. 

    I can see the 'this' reference being null when using Java Reflection.  I will concede this. But multithreaded has nothing to do with it. 

     

    See [url=http://pmd.sourceforge.net/rules/design.html#NonThreadSafeSingleton]this[/url], and [url=http://www.ibm.com/developerworks/java/library/j-jtp06294.html#2.4]this[/url], and by rerference, [url=http://www.ibm.com/developerworks/java/library/j-jtp0618.html]this[/url]. Threading has everything to do with it.



  • @ounos said:

    @TheRider said:

    @ounos said:

    I always start my java methods with:

    if (this == null) throw new NullPointerException();
    

    Catches many errors.

    Never ever have I stumbled upon a this being null in Java. Can you elaborate a bit about the situations where that may happen?

    It is very rare but painful to track. Sometimes you create an object in one thread and use it in another. Without proper locking "this" could be uninitialized. But you need to do this in every instance method of shared objects.

    I'm not weighing in on whether it could happen or not.  However, if it could happen at that point in the code, doing that won't protect you - without locking, it could happen immediately after your if check to see if it happened.  Sure, you've narrowed your window, because the time delta between processing your if and the next statement is probably less than the time delta between the method being invoked and the first statement in the method being processed.  However, there's still a window; you haven't fixed it.



  • Wow, these guys even better than me!



  •  @amischiefr said:

    I just found this code in some legacy crap I am trying to replace.  I am not sure what this person was thinking, but they obviously didn't test it.


        protected static ArrayList getIExecutableObjects(Connection con, IExecutable criteria)
        {
            if (criteria == null)
                return criteria.getDataBeanContainer();

            return executeSQL(criteria, con, criteria.getSelect());
        }

    An easy guess would be something like the python idiom

    def getIExecutableObjects(con, criteria)
        if not criteria:
            return criteria.getDataBeanContainer()

        return executeSQL(criteria, con, criteria.getSelect())



  • @beltorak said:

    See this, and this, and by rerference, this. Threading has everything to do with it.

    Actually all the examples you just gave has nothing to do with 'this' being referenced as null. The authors are refering to situations where a thread is attempting to implement non-static from an object when that object's constructor has not yet completed leading to unpredictable results.



  • @RoadieRich said:

    An easy guess would be something like the python idiom

    def getIExecutableObjects(con, criteria)
        if not criteria:
            return criteria.getDataBeanContainer()

        return executeSQL(criteria, con, criteria.getSelect())

    And what makes the Python version better?

    (seriously, I don't know)



  • @Zecc said:

    And what makes the Python version better?

    (seriously, I don't know)

    Since it would be python, it would only run in one thread so this/self can never appear as null.



  • Actually, I was refering to the

    if not criteria:
            return criteria.getDataBeanContainer()
    bit.

    I suppose you can call getDataBeanContainer on an empty string, or something like that?



  • @fyjham said:

    Yeah, I get that. I mean as in both methods were intended when it wasn't null. But since the first returns that wasn't feasible ;) Sorta like this:

    if (criteria == null)
    {
       return criteria.getDataBeanContainer();
       return executeSQL(criteria, con, criteria.getSelect());
    }

     

    I'm not sure what you are getting at with that example, it causes the compiler to throw an error about unreachable code.  Sorry, mayby I'm just slow this morning.



  • @beltorak said:

    @amischiefr said:

    @ounos said:

    True, if you only code in a single processor. "This" is just a register and its initialization can be reordered as a typical cpu optimization.

     

    I think that I just thought of the answer.  Err.. nevermind. 

    I can see the 'this' reference being null when using Java Reflection.  I will concede this. But multithreaded has nothing to do with it. 

     

    See this, and this, and by rerference, this. Threading has everything to do with it.

     

    Your first example talks about the singleton design where you are using a local variable which holds a reference to the singleton object being null, not a reference to 'this'.  Your second example talks about escaping the constructor and having problems storing a reference to 'this'  in a static variable, nothing about accessing 'this' from within a method. The third example talks about the same thing as the second: creating a race condition by escaping the 'this' reference in the constructor.

     

    These examples talk about why you shouldn't pass around the 'this' reference until the constructor has completed.  NONE of the examples say anything about accessing the 'this' reference while inside of a method, or say anything about it EVER being null within these methods.  All these examples talk about is how to be careful while constructing new objects and how not to create a race condition by accessing and passing 'this' while inside of the constructor.



  • @Zecc said:

    Actually, I was refering to the

    if not criteria:
            return criteria.getDataBeanContainer()
    bit.

    I suppose you can call getDataBeanContainer on an empty string, or something like that?

     

    In python, it's easy to overwrite the nonzero method on classes, to control their value in a boolean context, for instance:

    class Criteria(object):
        def nonzero(self):
            return self.hasDataBeanContainer()

        # other methods not shown



  • Thank you. That was the kind of information I was asking for and was too lazy to search.



  • @amischiefr said:

    @ounos said:


    True, if you only code in a single processor. "This" is just a register and its initialization can be reordered as a typical cpu optimization.

     

    I think that I just thought of the answer.  Err.. nevermind. 

    I can see the 'this' reference being null when using Java Reflection.  I will concede this. But multithreaded has nothing to do with it. 


    Can you elaborate on how you can use Java Reflection to see 'this' as null?



  • @ounos said:

    Can you elaborate on how you can use Java Reflection to see 'this' as null?
     

    No, I was thinking that possibly through reflection a class may not have an actual instance created in a static reference, thus causing a problem, but I can't duplicate it.

     

    Can you prove/show an example of it happening in a multithreaded environment?



  • @amischiefr said:

    @ounos said:

    Can you elaborate on how you can use Java Reflection to see 'this' as null?
     

    No, I was thinking that possibly through reflection a class may not have an actual instance created in a static reference, thus causing a problem, but I can't duplicate it.

     

    Can you prove/show an example of it happening in a multithreaded environment?


    No, but it happens all the time. Don't be lazy and put the check at the start of every instance methods, unless you want your code to randomly break when it goes to run on a multiprocessor. JDK source code does that too.



  • @ounos said:

    No, but it happens all the time.
     

    If it happens all of the time then you should be able to show an example, and quite possibly some logs indicating the error.  I mean, that is, if it happens "all of the time" like you say. :)

    @ounos said:

    Don't be lazy and put the check at the start of every instance methods, unless you want your code to randomly break when it goes to run on a multiprocessor. JDK source code does that too.
     

    I don't and I can say with confidence that I have never had this problem.  Ever.  As a matter of fact, I have NEVER seen anybody else's code that has this check in it either.

    Provide some proof of your claim or stop trolling.



  • Just make two threads, one thread constantly creating objects, second thread constantly reading them, invoking a method on them and checking whether this == null. In the first 5 minutes you'll get it, in a dual core machine.



  • @ounos said:

    Just make two threads, one thread constantly creating objects, second thread constantly reading them, invoking a method on them and checking whether this == null. In the first 5 minutes you'll get it, in a dual core machine.

    Interesting idea! Our production server is multi-core so I thought that would be the best place to try. Have been running for half an hour and still no sign of a null this, but terminal now seems very slow so maybe one's about to happen. Shall I keep going?



  • Did you use reflection?


Log in to reply