views:

97

answers:

6

I have a very odd problem.

A class property is mysteriously reset between method calls.

The following code is executed so the constructor is called, then the parseConfiguration method is called. Finally, processData is called.

The parseConfiguration method sets the "recursive" property to "true". However, as soon as it enters "processData", "recursive" becomes "false".

This problem isn't isolated to a single class -- I have several examples of this in my code.

How can this possibly be happening? I've tried initialising properties when they're declared outside any methods, I've tried initialising them in constructors... nothing works.

The only complication I can think of here is that this class is invoked by an object that runs in a thread -- but here is one instance per thread, so surely no chance that threads are interfering. I've tried setting both methods to "synchronized", but this still happens.

I'm on JDK 1.6.0_19 on linux.

Please help!

/**
 * This class or its superclasses are NOT threaded and don't extend Thread
 */
public class DirectoryAcquirer extends Manipulator
{
/**
 * @var Whether to recursively scan directories
 */
private boolean recursive = false;

/**
 * Constructor
 */
public DirectoryAcquirer()
{
}

/**
 * Constructor that initialises the configuration
 *
 * @param config
 * @throws InvalidConfigurationException
 */
public DirectoryAcquirer(HierarchicalConfiguration config) throws InvalidConfigurationException
{
    super(config);
}

@Override
protected void parseConfiguration() throws InvalidConfigurationException
{
    // set whether to recurse into directories or not
    if (this.config.containsKey("recursive"))
    {
    // this.recursive gets set to "true" here
        this.recursive = this.config.getBoolean("recursive");

    }
}

@Override
public EntityCollection processData(EntityCollection data)
{
// here this.recursive is "false"
    this.logger.debug("processData: Entered method");
}
}
A: 

Synchronized methods just means that two threads cannot be within the synchronized methods at the same time. It is useful for race conditions, but it is not the only way that unexpected things might happen.

The line

// this.recursive gets set to "true" here
    this.recursive = this.config.getBoolean("recursive");

confuses me. How do you know that this.config.getBoolean("recursive") returns true? It's only getting a boolean value stored with the key recursive. That value might be true, but it might also be false.

Assuming you verify that this.config.getBoolean("recursive") really does return true, then you have to look outside your code. Did something actually call protected void parseConfiguration()? If not, then this.recursive will have never been set beyond it's initial declaration.

Let's say that protected void parseConfiguration() was called, and it properly set this.recursive to true. Then you need to hunt through the rest of the class (an only this class because this.recursive is private, and see if any other code might reset the value.

Other possibilities are that you're not dealing with the same object between the two calls to protected void parseConfiguration(). Sometimes this happens when you use object maps or other forms of object caching which present the opportunity to switch out objects if they are used or written wrong. You might want to put in some logging which will print out the Object's id (toString()) along with the value of this.recursive.

Edwin Buck
I've confirmed the state of this.recursive using a debugger.I just tried debugging with the objectId -- it is the same objectWhat's odd is that in one of the other classes I was working on yesterday, instantiating an object outside any constructor failed, and the object (an ArrayDeque) was never initialised
Michael Jones
A: 

There is not sufficient context to explain what is going on, but I can guarantee that private attributes do not reset "spontaneously" in Java. If I were to guess at possible causes, I'd vote for one or more of the following:

  • the methods are not being called in the order you think they are,
  • one or of the methods is overloaded or overridden, or
  • you are looking at different objects.

For example, if a parseConfiguration method is called by the superclass, then it may be that the actual parseConfiguration method used is one in a subclass of DirectoryAcquirer.

Declaring the methods as synchronized is unlikely to help. At best, that will prevent two threads calling those methods simultaneously on the same object. It won't stop calls from another thread entirely.

Stephen C
I've added breakpoints to both methods, and parseConfiguration is always called before the other.There are only 2 places where the property could be set -- when it's initialised, or in parseConfiguration... so perhaps parseConfiguration isn't being called and there are 2 objects involved. However, I've just debugged, and the same object id occurs in both methods, so this can't be possible either.As for superclass methods being involved -- the property is private, and there are only 2 methods + 2 constructors in the class and nothing extends DirectoryAcquirer.
Michael Jones
Don't believe object ids that your application is generating. The best available surrogate for object identity is the object's identity hashcode value. Also, I'd use trace printing rather than a debugger for investigating this.
Stephen C
By the object identity hashcode do you mean what I get when I print "this.toString()" on the object? That's what I tried.What do you mean by trace printing? I've been logging to the console as well as using the debugger, and both suggest the same thing.
Michael Jones
A: 

I'd take an old-school approach.

Add this to parseConfiguration:

System.err.println("parseConfiguration: this = " + this + ", config = " + config
    + ", recursive = " + recursive + " on " + Thread.currentThread());

Add this to processData:

System.err.println("processData: this = " + this + ", config = " + config
    + ", recursive = " + recursive + " on " + Thread.currentThread());

It will tell you in what order they are being called and verify that they are referencing the same objects and doing so on the same threads.

The only causes I can think of are either threading (parseConfiguration and processData are being called on different threads) or, they are being called on different objects.

Devon_C_Miller
A: 

It really smells to me like the different method calls on the one object are being called from two different threads. This totally explains the 'mysterious reset'-like behavior.

In a nutshell, changes to member variables are not guaranteed to be seen between threads. Read up on the Java Memory Model for details.

To over come this, there are a couple of options. The two easiest (for your example) would be either a) use the volatile modifier:

private [volatile][2] boolean recursive = false;  

or b) use java.util.concurrent.AtomicBoolean

private [AtomicBoolean][3] recursive = new AtomicBoolean(false);

The synchronized keyword, while an important tool for multithreaded programming, is not something one can use properly without understanding how it works (and does not work) in greater detail. Try the above suggestions first.

Stu Thompson
A: 

I would try running this piece of code with a debugger and see what exactly is happening. Because there are a few simple scenarios for a mistake:

  • the methods are not doing what you expect
  • your this.recursive = true code is not reached at all
  • you are not setting the value to true at all
  • you are not checking the right property
  • you are not checking the property correctly

Debugger will help you with that. Please update information when you see it and you could even have an idea to help yourself.

Leni Kirilov
A: 

Thanks for your replies.

It seems that the problem was related to casting:

/**
 * 
 */
public class Manipulator
{
    // both constructors call parseConfiguration()

    protected void parseConfiguration() throws InvalidConfigurationException
    {
      // don't do anything. this is a default method that can optionally be 
      // overridden by subclasses
    }
}

I was referring in some places to the object as a DirectoryAcquirer, an at other times it was upcast to a Manipulator. I think that this was interfering so the empty parseConfiguration method was called after the child one was called -- or something like that.

Anyway, I made the parseConfiguration method in the superclass abstract, and now it's fine.

Michael Jones