Problems concerning Singleton pattern and threads..

This is where you talk about the NXJ software itself, installation issues, and programming talk.

Moderators: 99jonathan, roger, imaqine

Problems concerning Singleton pattern and threads..

Postby Gof » Thu Dec 13, 2007 1:31 pm

Hi

We are making a mapmaking robot, that drives round on the lego road elements, and builds a map. We have a (Danish) blog here regarding the project if anyone is interrested.

However we think we have found a bug in the Firmware / VM implementation on the NXT.

We have a class called Core, which implements some data that the robot needs to access from alot of places, so Core is implemented with a Singleton pattern:

Code: Select all
public class Core {
 private static Core singleton;

 private Data data;

 private Core() {
  data = new Data();
 }

 public Core getSingleton() {
  if( singleton == null) {
   singleton = new Core();
  }
  return singleton;
 }

 public Data getData() {
  return data;
 }
}


Now if we from our main method, call
Code: Select all
Core.getSingleton().getData()

and do something to this data (Like adding another point or whatever). And then from another thread (Later on as this is triggered by us, so we know the main thread is done accessing the data). Then
Code: Select all
Core.getSingleton().getData()

returns a new Data object, instead of, as expected the already created Data object in Core.

To us it looks like each thread gets its own "Singleton" which kinda ruins the idea behind Singleton pattern.

We are 99% sure this isnt a thread synchronization problem, as i said earlier, the second thread dosent run until we start it manually, so its not running at all in the beginning when the main thread is accessing the Core at first.

/Gof
Gof
New User
 
Posts: 6
Joined: Fri Aug 24, 2007 3:43 pm

Postby gloomyandy » Thu Dec 13, 2007 2:23 pm

Hi I'm not at home to try this at the moment but a few questions...
1. I assume that getSingleton is actually declared static in your real code?
2. Perhaps you should add another data method to the class (like an int), that you can change the value of off (that way we can see if the problem lies with the singleton or maybe with the Data object).
3. Have you tried checking the contents of the Data object from the same thread? Was it ok?

Andy
User avatar
gloomyandy
leJOS Team Member
 
Posts: 4084
Joined: Fri Sep 28, 2007 2:06 pm
Location: UK

Postby Gof » Thu Dec 13, 2007 3:10 pm

gloomyandy wrote:Hi I'm not at home to try this at the moment but a few questions...
1. I assume that getSingleton is actually declared static in your real code?
2. Perhaps you should add another data method to the class (like an int), that you can change the value of off (that way we can see if the problem lies with the singleton or maybe with the Data object).
3. Have you tried checking the contents of the Data object from the same thread? Was it ok?

Andy


1. Yeah, its static in the code, just a mistype :)

2. Well, the data object we have atm has an int inside it, with a simple getSize() method that just returns this int. And this size returns 6, in the main thread (After we add 6 objects to the data object), if we then call getSize() from our other thread, it return 0. But if we then do getSize() from the main thread again, its sill returning 6. So it would seem that the Class loader has loaded the Class for each thread, and therefor we are getting a static variable / thread accessing.

3. Yep we have tried changing the content of Data object, from the same thread, as far as we can tell, we are getting 2 diffrent data objects. 1 / thread, which shouldnt be the case.

/Gof
Gof
New User
 
Posts: 6
Joined: Fri Aug 24, 2007 3:43 pm

Postby gloomyandy » Thu Dec 13, 2007 4:40 pm

Hi,
I've just tried the following code...
Code: Select all
import lejos.nxt.*;
import lejos.nxt.comm.*;
class Core {
 private static Core singleton;

 private int data;

 private Core() {
  data = 0;
 }

 public static Core getSingleton() {
  if( singleton == null) {
   singleton = new Core();
  }
  return singleton;
 }

 public int getData() {
  return data;
 }
 
 public void setData(int val)
 {
    data = val;
 }
}

class myThread extends Thread
{
  public void run()
  {
    LCD.drawInt(Core.getSingleton().getData(), 5, 0, 2);
    LCD.refresh();
    Core.getSingleton().setData(42);
    LCD.drawInt(Core.getSingleton().getData(), 5, 8, 2);
    LCD.refresh();         
  }
}

public class Singleton {

 public static void main(String [] args) throws Exception {
   Core.getSingleton().setData(27);
   LCD.drawInt(Core.getSingleton().getData(), 5, 0, 1);
   LCD.refresh();
   try{Thread.sleep(5000);}catch(Exception e){}
   myThread t = new myThread();
   t.start();
   try{Thread.sleep(5000);}catch(Exception e){}   
   LCD.drawInt(Core.getSingleton().getData(), 5, 8, 1);
   LCD.refresh();   
}
}



It produces the expected values of 27 42, 27 42. So it looks like there is not a very basic problem with the singleton and threads. Perhaps there is some sort of issue with the Data object you are using?

Oh and to be safe if you are using threads like this then many of the methods need to be synchronized (particularly the getSingleton method)....

Andy
User avatar
gloomyandy
leJOS Team Member
 
Posts: 4084
Joined: Fri Sep 28, 2007 2:06 pm
Location: UK

Postby Gof » Fri Dec 14, 2007 9:15 am

gloomyandy wrote:It produces the expected values of 27 42, 27 42. So it looks like there is not a very basic problem with the singleton and threads. Perhaps there is some sort of issue with the Data object you are using?

After quite a few tests we have not yet been able to reproduce or isolate the issue outside of the system that we are building. Whether this is caused by some complex interdependency generating the erronous behaviour or just stupidity on our part is probably going to stay an open question due to the limited time we have available to debug issues like this.
The solution for us has been to drop the singleton pattern and instead just supply the core in the constructor arguments of any of the threads created that need to access it. This works in 100% of the cases, whereas the singleton pattern failed in 100% of the cases in our system (and worked in 100% of the cases in the simple testcases we could cook up).

Oh and to be safe if you are using threads like this then many of the methods need to be synchronized (particularly the getSingleton method)....

Of course, otherwise two different cores could be returned in the first evocation of the getSingleton method if several threads accessed it simultaneously. (synced in our code didn't do anything to help)
On a sidenote: Does lejOS fully support class-level locks? That is, syncronized static methods?
Gof
New User
 
Posts: 6
Joined: Fri Aug 24, 2007 3:43 pm


Return to NXJ Software

Who is online

Users browsing this forum: No registered users and 1 guest

more stuff