• Post Reply Bookmark Topic Watch Topic
  • New Topic

Singleton class private constructor woes  RSS feed

 
Pat Farrell
Rancher
Posts: 4686
7
Linux Mac OS X VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I've got an heirarchy of classes and one of them uses the usual Singleton pattern to make sure I have one and only one instance. I've got the usual double-check lock trick of a private constructor and the only private variable is a static instance of the

which in turn calls the private constructor.

I also have a daemon thread doing caching. Occasionally it gets an access exception such as



I've checked and there are no calls to the constructor and no explicit calls to newInstance() on the class.

I'd like to eliminate the exception, and I'd like to understand what is actually going on.
[ November 13, 2007: Message edited by: Jim Yingst ]
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Do any of the classes in the error message have any relationship with your singleton class? Right now it looks like you just started talking about a different topic midway through your post. Is there reason to believe your singleton is somehow connected with the error?

Also, isn't there any more to the stack trace?
 
Pat Farrell
Rancher
Posts: 4686
7
Linux Mac OS X VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The classes are direct subclasses. PersistantBusinessObject is the common superclass. There is a Facility class extending it. AFrameDefaultLocation extends Facility.

There are some reflection related classes in the stack dump. I left them out for clarity. The complete stack dump looks like this:



(BTW, I think this really is an Advanced problem, nearly everything posted in the advanced section is bounced out. Would be nice to see some seriously hard problems stay in advanced.)
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You were talking about a singleton. Now you're talking about three different classes. Is one of them a singleton? All of them? Should I just forget about the singleton? I'm still confused.

From your stack trace, it sounds like you should look at PersistentBusinessObject.java line 135. That should be what's calling the newInstance() method. If not, then what is at that line?

Regarding Advanced: yeah, we really do have a shortage of even halfway-decent problems in that forum. I'm not sure why this one was bounced. Maybe an overly-quick read of the topic led to a misunderstanding? I dunno.

Incidentally your mention of the "double-check lock trick" concerns me a bit. Using a private constructor and a private static variable is pretty much the standard way to make a singleton, regardless of whether double-checked locking is used. And double-checked locking itself has long been inherently broken. It finally got fixed if and only if you're using JDK 5 or later and have declared the static field to be volatile. Hearing it mentioned in that offhand manner makes me wonder if you're using double-checked locking at all, and if so, is it being done correctly?
 
Pat Farrell
Rancher
Posts: 4686
7
Linux Mac OS X VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Jim Yingst:
[QB]You were talking about a singleton. Now you're talking about three different classes. Is one of them a singleton? All of them? Should I just forget about the singleton? I'm still confused.


There is a tree of classes. The Facility is normal, its places, buildings, etc. There is one class, and one instance of AFrameDefaultLocation, which is a special Facility.


From your stack trace, it sounds like you should look at PersistentBusinessObject.java line 135. That should be what's calling the newInstance() method. If not, then what is at that line?


That is the newInstance() call that is blowing up.
It should not be calling it, but I can't figure out why.
The data in the Singleton instance is common, you can't construct one.



Regarding Advanced: yeah, we really do have a shortage of even halfway-decent problems in that forum. I'm not sure why this one was bounced. Maybe an overly-quick read of the topic led to a misunderstanding? I dunno.


No tracks on who moved it? can you move it back?



Incidentally your mention of the "double-check lock trick" concerns me a bit. Using a private constructor and a private static variable is pretty much the standard way to make a singleton


No, I'm not using the usual lame and broken approach.
I'm using the only known working approach, which has a static private class inside the Singleton instance that constructs one instance of the wrapped class. The internal HelperSingleton is completed, which makes sure that the private constructor is called and completed.

See IBM's discussion of Double-checked locking

-- edited: on -> one
[ November 12, 2007: Message edited by: Pat Farrell ]
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
[Pat]: There is a tree of classes. The Facility is normal, its places, buildings, etc. There is one class, and one instance of AFrameDefaultLocation, which is a special Facility.

So are you saying that AFrameDefaultLocation is the singleton that you were previously talking about? Is that the class that has the private constructor?

[Jim]: From your stack trace, it sounds like you should look at PersistentBusinessObject.java line 135. That should be what's calling the newInstance() method. If not, then what is at that line?

[Pat]: That is the newInstance() call that is blowing up. It should not be calling it, but I can't figure out why.


Well, previously you said "I've checked and there are no calls to the constructor and no explicit calls to newInstance() on the class." Now, apparently, a call to newInstance() has been found on line 135 of PersistentBusinessObject.java. If you're wondering why that is being called, I would have to guess that it's because it's being called from DatabaseCache.java line 115. Which in turn is being called from WorkingSetCache.java line 140.

I guess I'm still not understanding what the question is here. Is the problem that you don't understand how it is that the newInstance() method is being called? Or why it is that you should not call newInstance() on a class with a private constructor? Or are you trying to find a workaround to, yes, call newInstance() (or something like it) on a class with a privzte constructor?

[Pat]: No tracks on who moved it? can you move it back?

Not easily accessible. I can move it back, but at the moment I'd prefer to wait until I understand what it is you're asking. Before I countermand another mod's action, I prefer a better understanding of what's going on.

[Pat]: I'm using the only known working approach, which has a static private class inside the Singleton instance that constructs one instance of the wrapped class. The internal HelperSingleton is completed, which makes sure that the private constructor is called and completed.

See IBM's discussion of Double-checked locking


Well, I know of an approach that uses a private static helper class inside the singleton - but (a) it's not discussed in that article that I can see, and (b) it's not double-ckecked locking. Rather, it's an alternative to double-checked locking, called the Initialization on demand holder idiom. It sounds like that might be what you're referring to here. Regardless of terminology though, it sounds like you're aware that traditional double-checked locking has issues, so good - that was my main concern here. This is probalby an unimportant side issue.
 
Pat Farrell
Rancher
Posts: 4686
7
Linux Mac OS X VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Jim Yingst:

So are you saying that AFrameDefaultLocation is the singleton that you were previously talking about? Is that the class that has the private


yes.


I guess I'm still not understanding what the question is here. Is the problem that you don't understand how it is that the newInstance() method is being called? Or why it is that you should not call newInstance() on a class with a private constructor? Or are you trying to find a workaround to, yes, call newInstance() (or something like it) on a class with a privzte constructor?


I don't understand how it is being called.
I fully understand why it is failing, the error message is technically accurate. I understand that its not supposed to be called.

There are no explicit calls to newInstace() for the class, so its some sort of dymanic bug.

I expect its in my code, but I can't find it.

So I asked in the advanced section so some pros could help me track it down.



Rather, it's an alternative to double-checked locking, called the Initialization on demand holder idiom..[/QB]


whatever its called, its what you find googling for serious fixes for the usual double-check bug farm.

Sadly, Singletons have many other problems in the Container and J2EE space. Yet another (off topic) reason I dislike J2EE
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
[Pat]: I don't understand how it is being called.

And so I come back to the stack trace, specifically:

Is this code that you have access to, or not? Personally I have no way of knowing what this code does (other than guessing based on the method and class names) but this is where I would look to understand why and how this code is being called.
 
Pat Farrell
Rancher
Posts: 4686
7
Linux Mac OS X VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Jim Yingst:

Is this code that you have access to, or not? Personally I have no way of knowing what this code does (other than guessing based on the method and class names) but this is where I would look to understand why and how this code is being called.


Oh, yes. I wrote that code.
What it is supposed to do is get all the objects in the cache, see if any are grody, throw those away, refresh the recently used ones from the database.

There is no higher code, its a daemon that runs periodically and makes a pass doing the right thing. (or blowing up when its buggy).

I think the problem is that for some reason, its trying to recreate the singleton, rather than just using it.

I'll just plunk around it in some more, add some PMD and do the usual debugging stuff.

I was just looking for some insight in general.
 
Nicholas Jordan
Ranch Hand
Posts: 1282
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
can we see the full codebase for:
 
Pat Farrell
Rancher
Posts: 4686
7
Linux Mac OS X VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yeah, I can post it. I wrote it to be under some suitable open source license, like BSD or Apache.

Its straight out of Peter J Denning's papers. He was one of my advisors when I was studying for my PhD in Computer Science, he was the CS Department Chair at the time.

I'll check out the source and put it up on my personal website in a bit
 
Ernest Friedman-Hill
author and iconoclast
Sheriff
Posts: 24217
38
Chrome Eclipse IDE Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You could post that, but it's not going to help Jim, or I, to help you. On Nicholas' planet, things might be different, I dunno. But I wouldn't count on it.

The code that's going wrong is in PersistentBusinessObject.java, around line 135. That's what we need to see: the factoryFromQueryString() method. If we can see that, I suspect we'll have an "aha!" moment pretty quickly.
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well, if Pat is trying to understand how and why the newInstance() is being called, the answer is really a combination of the various methods and classes in the stack trace:

The most proximate cause would be in PersistentBusinessObject.factoryFromQueryString() - but it may be that the part that Pat needs to understand is in DatabaseCache.processAll() or WorkingSetCache.run(). Part of the question is, why is Pat not expecting this code to be called? Which part of the code is he expecting to behave differently?

[Pat]: I think the problem is that for some reason, its trying to recreate the singleton, rather than just using it.

Yes, that sounds plausible. It's definitely trying to create an instance. Is ther ecode somehwere that is supposed to detect whether an instance is already available, without creating it? Where is that code? Maybe it's got a bug.
 
Ernest Friedman-Hill
author and iconoclast
Sheriff
Posts: 24217
38
Chrome Eclipse IDE Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Jim Yingst:
Well, if Pat is trying to understand how and why the newInstance() is being called,


Up at the top of the thread he says


I've checked and there are no calls to the constructor and no explicit calls to newInstance() on the class.


so I was going with the assumption that the call to newInstance() was coming in in some surprising way, and that this is where the problem lay.
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
[EFH]: Up at the top of the thread he says

I've checked and there are no calls to the constructor and no explicit calls to newInstance() on the class.


Yes, and then later when I asked about PersistentBusinessObject.java line 135 he says

That is the newInstance() call that is blowing up. It should not be calling it, but I can't figure out why.


So apparently now there is a newInstance() call where before there wasn't. I don't know why "it should not be calling it", but the answer may be at PersistentBusinessObject.java line 135, or it may be higher in the call stack. Certainly PersistentBusinessObject.java line 135 seems a good place to start, but we may well end up needing more context.
 
Pat Farrell
Rancher
Posts: 4686
7
Linux Mac OS X VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
the joys of threaded snippets of code. I wrote the code with the newInstance(). what I don't understand is why it is being called on an object that should never be duplicated. Its a singleton, it is supposed to be the one and only true love.

Its the call, or specifically why it is being called occasionally and blowing up when for hours or days it works perfectly.

probably a data triggered, relating to order of data in the database, which changes over time.

I'll focus on the order of records in the database, and add some PMD

Thanks
 
Pat Farrell
Rancher
Posts: 4686
7
Linux Mac OS X VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Is there code somehwere that is supposed to detect whether an instance is already available, without creating it? Where is that code? Maybe it's got a bug.[/QB]


No, I had not done that level of checking, its supposed to be a Singleton :-)

Clearly I have a bug someplace. If it was easy to find, I would have found it by now :-)
 
Nicholas Jordan
Ranch Hand
Posts: 1282
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
[PF]I'll check out the source and put it up on my personal website in a bit
Ok, I am eager to work on this because of the caliber of work involved, I will dig through the wesite this weekend when I have sufficient time to be effective and look at the work of the cited Peter J Denning's papers

[message edit: well if the cache is being maintained in a multi-threaded application, it seems obvious that we should start by insuring the cache makes all entries synchronized. double-checked locking pattern is broken unless you use explicit memory barriers, which I do not expect to find in Java, even with volatile: Lots of very smart people have spent lots of time looking at this. There is no way to make it work without requiring each thread that accesses the helper object to perform synchronization. See: The "Double-Checked Locking is Broken" Declaration , in that page Alexander Terekhov came up with a clever solution to your woes that I think is worth attempting.

Parallel operations must be expressed in parallel syntax. Otherwise the compiler will treat them as "soft real-time" which means "Que sera sera" by real-world experience. Decomposition of one's apps into a multiplicity of threads corresponding to separate regions of program logic is a complicating artifact necessitated by schedulers (usually operating systems) that fail to accommodate arbitrary design constraints for an individual application.

There was a compiler switch posted at Java World given to me by a javarancher that specifies no JIT compiler, but I have lost track of that switch due to not having my cat at the time. {don't ask   :roll:  } (it i not the switch I emailed yesterday g:none which should have been -Xint ) If this increases robustness, then we may have isolated an unexpected thread interleaving ]

[PF:]the joys of threaded snippets of code. I wrote the code with the newInstance(). what I don't understand is why it is being called on an object that should never be duplicated. Its a singleton, it is supposed to be the one and only true love.


Is your challenge intended to run portably ? And if not, can you get the headers for the JVM build your app will run under and is testing being done on the actual operational platform or a tweaked development platform.


[EFH:]On Nicholas' planet, things might be different
Which planet ?

[ November 16, 2007: Message edited by: Nicholas Jordan ]

[ November 18, 2007: Message edited by: Nicholas Jordan ]
[ November 20, 2007: Message edited by: Nicholas Jordan ]
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!