iOS Subtracting two numbers gives EXC_BAD_ACCESS

[blueeye]

Hi MacRumours,

I'm writing an iPhone app which uses the accelerometer to control another external application (just for some context).

In order to calculate the difference between the previous accelerometer value and this one I use the following two lines (I only need X and Y):

float diffx = ([acceleration x] - [prevAccel x])*100;
float diffy = ([acceleration y] - [prevAccel y])*100;

This works most of the time, but sometimes (either randomly or when the difference becomes large, mostly the latter) I get EXC_BAD_ACCESS on the first of the two lines above.
In GDB I tried the following:

(gdb) p acceleration
$3 = (UIAcceleration *) 0x128500
(gdb) p acceleration.x
$4 = 0.869384765625
(gdb) p acceleration.y
$5 = -0.036224365234375
(gdb) p acceleration.z
$6 = -0.543365478515625
(gdb) p prevAccel.x
$7 = 4.6495020515960297e-315
(gdb) p prevAccel.y
$8 = 1.7286328172809227e-38
(gdb) p prevAccel.z
$9 = 0
(gdb) p (acceleration.x - prevAccel.x)
$10 = 0.869384765625

after I got the EXC_BAD_ACCESS code and as you can see, the sum works...

I then tried the following:

(gdb) continue
Continuing.
Program received signal:  “EXC_BAD_ACCESS”.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x0000000f
0x31ec3ebc in objc_msgSend ()
(gdb) x/s $r1
0x34370cc4 <__PRETTY_FUNCTION__.55878+13008>:	 "x"

I'm not entirely sure what to do here since it works most of the time. Any ideas would be much appreciated.

 

[dejo]

How are you retaining prevAccel?

 

[blueeye]

How are you retaining prevAccel?

It's not retained explicitly. I tried copying the acceleration as follows:

if(prevAccel == nil) {
	prevAccel = [acceleration copy];
} else {
...

but it didn't work so I've just got

prevAccel = acceleration;

 

[robbieduncan]

It's not retained explicitly. I tried copying the acceleration as follows:

if(prevAccel == nil) {
	prevAccel = [acceleration copy];
} else {
...

but it didn't work so I've just got

prevAccel = acceleration;

So if you don't retain it and it's autoreleased what happens to it at the end of the current runloop? Normally it'll get released and next time in you'll access memory you no longer own and bang! Sometimes you'll get lucky, other times you won't.

 

[blueeye]

How are you retaining prevAccel?

Fabulous, I think I've fixed it by assigning prevAccel as follows:

prevAccel = [acceleration retain];

Thanks for pointing me in the right direction

 

[robbieduncan]

Fabulous, I think I've fixed it by assigning prevAccel as follows:

prevAccel = [acceleration retain];

Thanks for pointing me in the right direction

Remember to release it after you are done with it otherwise you are leaking memory...

 

[blueeye]

So if you don't retain it and it's autoreleased what happens to it at the end of the current runloop? Normally it'll get released and next time in you'll access memory you no longer own and bang! Sometimes you'll get lucky, other times you won't.

I guess it was that "Sometimes you'll get lucky bit" that threw me off the scent. I'm still relatively new to the concept of memory management.
Thanks for the help

Remember to release it after you are done with it otherwise you are leaking memory...

Done. Thanks again. I would have missed that otherwise.

 

[dejo]

Remember to release it after you are done with it otherwise you are leaking memory...

Does that include releasing it before reassigning it? (Sometimes memory management confuses even me.)

 

[blueeye]

Does that include releasing it before reassigning it? (Sometimes memory management confuses even me.)

I released before reassigning and it hasn't broken but I'm not entirely sure if it's correct either. It makes sense (to me) to do it that way but I'm hardly an expert memory manager.

 

[PhoneyDeveloper]

Few points.

I haven't used the accelerometer but a quick look at the header shows that it doesn't implement the NSCopying protocol so it won't copy correctly. This is arguably a bug but at any rate you can't expect the copy method to work correctly.

You probably want to add a retain property for your prevAccel ivar. That will make your memory management easier.

self.prevAccel = acceleration;

will retain the new value and release the old value.

When you see crashes in objc_msgSend they usually mean that you've tried to access an ivar where you failed to retain it.

 

[robbieduncan]

Does that include releasing it before reassigning it? (Sometimes memory management confuses even me.)

Yes. Otherwise the object you retained and failed to release will hang around until the app quites. If you retain an object you must release it. Assigning a new object to the pointer does not release the original object you retained...

 

[blueeye]

Few points.

I haven't used the accelerometer but a quick look at the header shows that it doesn't implement the NSCopying protocol so it won't copy correctly. This is arguably a bug but at any rate you can't expect the copy method to work correctly.

You probably want to add a retain property for your prevAccel ivar. That will make your memory management easier.

self.prevAccel = acceleration;

will retain the new value and release the old value.

When you see crashes in objc_msgSend they usually mean that you've tried to access an ivar where you failed to retain it.

As it stands I've simply assigned it to [acceleration retain], although prevAccel is assigned as (nonatomic, retain).

 

[dejo]

As it stands I've simply assigned it to [acceleration retain], although prevAccel is assigned as (nonatomic, retain).

If prevAccel is already a property with (nonatomic, retain), then just assigning it

prevAccel = [acceleration retain];

is not taking advantage of the property you have setup. As PhoneyDeveloper has suggested, you should be using

self.prevAccel = acceleration;

instead.

 

[blueeye]

If prevAccel is already a property with (nonatomic, retain), then just assigning it

prevAccel = [acceleration retain];

is not taking advantage of the property you have setup. As PhoneyDeveloper has suggested, you should be using

self.prevAccel = acceleration;

instead.

Ahh! I see. I'd misunderstood and thought that writing

prevAccel

was a convenience method of accessing

self.prevAccel

Thanks for clearing this up.

 

[dejo]

Ahh! I see. I'd misunderstood and thought that writing

prevAccel

was a convenience method of accessing

self.prevAccel

Thanks for clearing this up.

Using just prevAccel accesses the instance variable directly and doesn't use the getters or setters created via the @synthesize part of your property definition.

 

[blueeye]

Using just prevAccel accesses the instance variable directly and doesn't use the getters or setters created via the @synthesize part of your property definition.

I'll definitely keep that in mind then.
When I try using

self.prevAccel = acceleration;

Do I still have to using [acceleration retain] or is it sufficient that prevAccel has retain set?

 

[robbieduncan]

I'll definitely keep that in mind then.
When I try using

self.prevAccel = acceleration;

Do I still have to using [acceleration retain] or is it sufficient that prevAccel has retain set?

You do not.

self.prevAccel = acceleration;

is equivalent to

[self setPrevAccel:acceleration];

The synthesized code for setPrevAccel will look something like

-(void) setPrevAccel:(UIAcceleration) newValue
{
if (prevAccel!= newValue) {

    [prevAccel release];

    prevAccel= [newValue retain];

}
}

As you can see this releases any existing value set for the property and retains the new value for you (this assumes you declared the property as retain)

 

[blueeye]

@robbieduncan:

Okay, that's brilliant. Thanks for explaining that to me.