promet / PRAugmentedReality

iOS Augmented Reality Framework
http://praugmentedreality.com/
MIT License
606 stars 162 forks source link

Issue with using the PRARManager multiple times #53

Open esbenr opened 9 years ago

esbenr commented 9 years ago

S, using the video introduction to implement your library I have run into a problem when it comes to reuse.

I have a ViewController (VC) set up like the single view in your tutorial. On the touch of a button I am presenting this VC modally. Everything works flawlessly the first time.

When dismissing the VC, the VC gets deallocated. Since the PRARAManager is dispatch_once it doesent.

So the next time I create and present the VC again the + (id)sharedManagerWithSize:(CGSize)size andDelegate:(id)theDelegate gets called again. Only this time the manager has already been instantiated and therefore _sharedManager = [[self alloc] initWithScreenSize:size withRadar:NO andDelegate:theDelegate]; inside the initializer doesent get called, setting the delegate.

The state you have now is the PRARManager with a delegate that points på "the old" host VC - which is now deallocated. And the delegate to the new host VC is abandonned. This makes the PRARManager callback to a delegate that doesent exist and therefore not initialize correctly.

This is obvious if you try to debug the initialization method. The 2nd time it jumps over the initialization.

+ (id)sharedManagerWithSize:(CGSize)size andDelegate:(id)theDelegate
{
    @synchronized([LocationMath class]) {
        dispatch_once(&onceToken, ^{
            _sharedManager = [[self alloc] initWithScreenSize:size withRadar:NO andDelegate:theDelegate];
        });
    }

    return _sharedManager;
}

A solution for this would be to not make the PRARManager single instance and allow other host VC's to tear it down/deallocate it. To be honest I don't see the usage of a singleton instance. Running multiple instances don't make any sense.

The reason why your example is actually working is either:

  1. By pure luck the memory address gets reused when creating a new host VC the delegate, by coincidence reuses the memory address for the new VC an the PRARManager is luckily calling back to the right host VC.
  2. OR the Storyboard actually holds on to the host VC and never dealloc it = actually creating a memory leak.

I think you should change to a regular instance of PRARAManager. A singleton instance that holds a state is a bad practice, as it easily can go wrong.

I can fork your repo or do you take pull requests?

wtimme commented 7 years ago

The project once had a sharedManager instance, but it was removed with e102d4d17a05290356ed95880373a09ccaf5b843.