ARC compatibility and Invalid Email Address crash

[GoogleCodeExporter]

Hi all,
just to let you know that I've updated the SKPSMTPMessage classe and Extensions 
to make
the library compliant to ARC in iOS5.

Also, I happened to track down a bug that caused the application to terminate 
with EXC_BAD_ACCESS.
That only occurred when trying to send emails to invalid addresses (ie: 
yourname.hotmail.com): apparently,
the parser did not enlist the SMTP response code: 501 5.5.4 Invalid Email 
Address as a possibility,
which caused the program to keep running but  without a new StartWatchDog call, 
and a subsequest bad memory access at the end of the run loop when trying to 
deallocate the timer object.
I've changed the code to deal with this event.

If one of the developers could kindly point me towards the best place for me to 
upload the revised files,
I would be happy to share those with people using SKPSMTP under ARC developing.

Original issue reported on code.google.com by mmanni.dev on 21 Dec 2011 at 12:26

[GoogleCodeExporter]

The post should have probably gone under Enhancements, not defect. 

Original comment by mmanni.dev on 21 Dec 2011 at 12:28

[GoogleCodeExporter]

Can you attach it to a comment so I can use it please?

Original comment by DenisO...@gmail.com on 28 Dec 2011 at 2:41

[GoogleCodeExporter]

There you go...you need to replace some of the files provided in the original 
bundle with the ones here attached.
Let me know if you have any problem

Original comment by mmanni.dev on 28 Dec 2011 at 4:28

Attachments:

• [SKPSMTP Classes - ARC.zip](https://storage.googleapis.com/google-code-attachments/skpsmtpmessage/issue-56/comment-3/SKPSMTP Classes - ARC.zip)

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Thanks for that. It's implemented fine except the delegate was getting released 
early so I wasn't getting any success or failure messages. I changed it from:

@property (nonatomic, weak) id <SKPSMTPMessageDelegate> delegate;

to 

@property (nonatomic, strong) id <SKPSMTPMessageDelegate> delegate;

and I now get them fine. Let me know if you see any problem with this.

Original comment by DenisO...@gmail.com on 28 Dec 2011 at 4:47

[GoogleCodeExporter]

With ARC, it is good practice to always declare a delegate to be weak in order 
to avoid retain cycles (which happens, for instance, when A retains B and 
vice-versa). Making the delegate strong may be a workaround for your specific 
situation, but be aware that it may leed to memory leaks / objects not being 
deallocated at all. If I were you, I would instead have a look at the reason 
why the delegate is released to early in your specific procedure and tweak the 
code to handle that: it may be due to the customisation of the code to my 
specific needs (have a look at the conentionTimer target method in 
SKPSMTPMEssage.m) to address a BAD_ACCESS problem I had when the timeout method 
tried to reference the  SKPSMTPMessage instance that had already been released 
after the success / failure message. Hope that helps!

Original comment by mmanni.dev on 28 Dec 2011 at 5:24

[GoogleCodeExporter]

Hello Manni,
              Looks like this ARC Code leaks a lot of memory. Could you look into that. The CFHost and streams are getting retained both on the device and simulator. 

Ian, if you are reading this, you help will be appreciated. The current base 
current code "27 ?" seems to crash in you are building in the latest  SDK 
(non-beta).

Thanks 
Real.

Original comment by ireal....@gmail.com on 5 Feb 2012 at 3:14

[GoogleCodeExporter]

Hi there, yes you are right. The one uploaded was a quick beta and it has 
various issues: I will attach the refined version tonight alongside with a 
comment on a single memory leak in one of the CoreFoundation methods I can't 
seem to get rid of.

Original comment by m_mann...@hotmail.com on 5 Feb 2012 at 3:43

[GoogleCodeExporter]

Please find attached the new commit with multiple issues fixed.
There is still one memory leak I am fighting against: if you run the Leaks 
tool, you will notice that the function:

CFStreamCreatePairWithSocketToCFHost

in  NSStream+SKPSMTPExtensions generates a leak. There is also a comment on the 
line explaining why that should not happen, so I will be pleased if you could 
have a look into that and give feedback should you find the solution for the 
problem.
Thanks

Original comment by m_mann...@hotmail.com on 5 Feb 2012 at 4:28

Attachments:

• SMTPSender-ARC.zip

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Manny,
             Thanks for the great wok. 

Sorry for the delay. I did find a few lot of leaks  on this one too. You beat 
me to the bridge_transfer in the NStream extension. I was trying to compile for 
ARC without GC ( Target : IOS 4.3+ ) .  So I had to make the delegate 
unsafe_unretained.  Don't  know if this is done right but do look into this .

I see a lot of bridge_retain with the "write" part. Are you sure yo want to 
retain those? Could you tell me why we need to do that? Im just getting my head 
wrapped around this one.

At this point my code as some good number of leaks but seems to point to 
Security framework the sslOptions do get released. I get feeling he stream is 
not fully release somewhere a thread is probably still running. neat one with 
stopWatchdog call in the dealloc.

Manny, If any of this is wrong please do tell me what is the better option and 
why? Looking forward to head from you on the right and wrong things in that  
code. 

Nagging feeling sslOptions is not getting released or stream is not getting 
releases. Mycode leaks under Security framework. CFhost and the likes disappear 
though.

Really looks forward to be hearing about the next update from you.
regards
Real

Original comment by ireal....@gmail.com on 7 Feb 2012 at 1:36

Attachments:

• SKPSMTPMessage.m

[GoogleCodeExporter]

Hello mmanni,

To start with, forgive my typos in the previous message.
                        Here is my update again on the test of the new code. Did some more tests. This time I took the 
static lib and compiled it in. 

Details: Ran a test agains the code I supplied above along with your changes in 
dealloc and some of my own to SKPSTMP sans the "bridge_rtained" for the streams 
and your cool changes to the Stream additions. At this point the last version 
of SKPSTMP  I sent seems to micmic the static lib of Ian ( 8-9) leaks all in 
Security Framework again - 'mp_init' and 'init'.  Looks like we have nailed it 
- at least for a good bit.  

I still wonder if we can fix that leak? What if something in Ian's original  
code is actually causing the leak? I also wonder why a system framework would 
leak.  

Could you please look at the code and give me some feedback please.

Base SDK 5.0 target mixed use 4.3 and 5.x. 

For the static build I added Ian's NSDATA addtions to the target project and 
added to build targets one for simulator and one for the device.

The scene is cycle root is that it shows a 16 byte item printing to a non-ivar  
of 128 Bytes. Looks to me as if it is from the read operation.

Please advice.

regards
iReal.

Original comment by ireal....@gmail.com on 8 Feb 2012 at 9:52

[GoogleCodeExporter]

Hello iReal,
sorry but I've been (and still am) terribly busy and I didn't have time to look 
into the code in the detail yet.
As for the question about bridging, I followed these simple guidelines for 
migrating CoreFoundation objects to and from ARC (as you know ARC does not take 
care of Objects created from core Foundation, so we somehow need to tell it how 
to manage ownership of Objective-C objects bridged to CF ones and vice-versa):

• When changing ownership from Core Foundation to Objective-C you use CFBridg-
ingRelease().
 • When changing ownership from Objective-C to Core Foundation you use CFBridg-
ingRetain(). 
• When you want to use one type temporarily as if it were another without 
owner-
ship change, you use __bridge.

Now, I may have overlooked something in the ownership change, so feel free to 
point that out to me. 
And yes, if you target iOS 4 you should make all weak properties 
unsafe_unretained instead, as you correctly pointed out.

In terms of the possibility of Ian's original coding being leaking itself, I 
have no certainty. I can tell you that I've found some other people having 
memory issues with CF call to CFStreamCreatePairWithSocketToCFHost, and that 
there was a workaround for that under OSX and garbage collector (unfortunately 
that was not our case).

If the function leaks internally, than there is not much we can do about it, 
and also it would not constitute an issue for me as it is widely used as it is 
in the framework. If, anyways, the leak is caused by something overlooked 
during the ARC porting process, well that is different. Where do you exactly 
still have memory leaks? Try to run Instruments with Leaks and to nail down the 
leaks to the specific call: if out codes are on the same line, you should only 
have problems with the aforementioned CF function.

Original comment by mmanni.dev on 11 Feb 2012 at 3:57

[GoogleCodeExporter]

Hello Mmanni,

Great to hear from you. Thanks a ton for responding and guiding me.

 It is interesting what you point out. Correct me if I am wrong, since we really are not transferring the ownership to the "CF" family in those cases rather leting "CF" use it for as long as needed and our iVars should maintain ownership right? So _bridge ( do not alter ownership )  - keep with classes iVars should be in force right, rather than  bridge_tranfer ( take it I want no part of it ). For the purposes of its use CF does get a pointer to the item. That should suffice right?

I get the point you mention about us not being able to fix the "IOS Core 
Frameworks". I was only wondering if this will cause a potential users app to 
get rejected because the framework leaks.

I ran through several rounds of debugging and profiling. It looks to me that 
after upgrading the stream to SSL/TLS the framework leaks. This is  under 
"Responsible Library" =>"Security" "Responsible Frame"=>"init" and "mp_init". 
Apparently we have no control over that. So is this a "Kaput" for a good piece 
of code because of framework bug? I hope not. 

I will keep at it as time becomes available. Please do guide me further. I just 
getting this hang of this.

Thanks a lot again.
regards
iReal.

Original comment by ireal....@gmail.com on 11 Feb 2012 at 7:31

[GoogleCodeExporter]

Hello Mmanni,

Here is an update.  I confirm my suspicion. The leaks happens only on SSL . Non 
SSL and iOS5 ... no leak.

I look forward to your kind advice.

regards
iReal

Original comment by ireal....@gmail.com on 17 Feb 2012 at 5:53

[GoogleCodeExporter]

Hi,

I can see there has been a bunch of work done by you guys to get the code to 
work with ARC and solve memory leaks, but it doesn't look like the latest code 
with these changes is available.

Are there plans to offer this updated code?  Can I get a copy of the latest 
code?

Thanks

~Randy

Original comment by rd.pi...@gmail.com on 1 Apr 2012 at 5:17

[GoogleCodeExporter]

Randy, 
         I dumped it but let me see if I can find a copy.
Thanks 

Original comment by ireal....@gmail.com on 24 Nov 2012 at 5:41

[GoogleCodeExporter]

If this can help somebody, I just updated SKSMTPMessage files:
- adding comments and logs to help me understand the whole operations;
- splitting properties between public and private to feed only public 
properties with mail data from a plist;
- changing stream length test to reflect issue #53
- changing TLS to SSL to reflect issue #58 and Apple Tech Note 2287
- adding better multiple recipients management from stack overflow question 
5799112
- adding prefix check 550 and 553 from issue #23
- coming back to some Ian Baird code fragments regarding fired timers due to 
useless or inadequate events triggered.

No failure at this time.

Philippe.

Original comment by phbar...@gmail.com on 11 Dec 2012 at 10:54

Attachments:

• SKPSMTPMessage.h
• SKPSMTPMessage.m