Hi, I've fixed a few crashes while trying to compile iOS-Couchbase-Demo #226

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants

I've changed order of a few lines in -[TDRouter finished] because onFinished was already released by ARC in -[TDRouter stopNow]. Fixes a crash which prevented demo from even starting.

TDReplicator's property remote should be copied because it's value could be instantiated in another thread and released there. Also caused crashes.

+[TD_DatabaseManager initialize] also fixed, see comments there (also was stably crashing for me without this fix).

I've also updated CocoaPods spec for TouchDB and that needed bumping version to 1.03. Sorry for that, but I saw no way of testing demo's podfile without this.

Btw, another pull request is coming for iOS-Couchbase-Demo itself.

Owner

snej commented Jan 22, 2013

TDReplicator's property remote should be copied because it's value could be instantiated in another thread and released there. Also caused crashes.

This doesn't make sense to me — _remote is an NSURL, and NSURL instances are immutable, so -copy and -retain do the same thing. So this change shouldn't make any difference. The situation you describe isn't a problem because _remote will be retained by the replicator, so if it's released on another thread it still has a reference to it.

snej commented on 676ff62 Jan 22, 2013

This change isn't necessary. If you look closely at the StackOverflow thread you linked to, you'll see that the OP updated his post to say that the real problem was that one of his configurations was building without ARC.

With ARC the -copy call will have no effect because (a) NSCharacterSet is immutable, and (b) the compiler will see it and omit the -retain it would otherwise have generated when assigning to the strong property, so the refcount will be the same as it would have been.

snej commented on 5bfcc9c Jan 22, 2013

Where does TouchDBVersionNumber get declared, then? It's used farther down in the +versionString method, but you've removed the place it's declared. (And why is this necessary?)

Owner

explicitcall replied Jan 22, 2013

ok, I've got it, latest available podspec has this:

    if path =~ /TDServer\.m/
      f.puts "const unsigned char TouchDBVersionString[] __attribute__ ((used)) = \"@(#)PROGRAM:TouchDB  PROJECT:TouchDB-#{version}\";"
      f.puts "const double TouchDBVersionNumber __attribute__ ((used)) = (double)#{version};"
    end

but TDServer.m is not available at all in 1.01. Where and in which way TouchDBVersionNumber should be defined at all? Because TouchDB_vers.c isn't generated when building from CocoaPods.

Owner

explicitcall replied Jan 22, 2013

#ifndef COCOAPODS_BUILD was introduced essentially to solve this exact problem: in my updated podspec `static double TouchDBVersionNumber = #{version}' was written directly, like this:

if path =~ /TDRouter\.m/
    f.puts "static double TouchDBVersionNumber = #{version};"
end

because TouchDB_vers.c isn't generated when building from CocoaPods. Making this #ifndef and code injection is much easier than transplanting TouchDB_vers.c generation from origin Xcode project file.

snej replied Jan 22, 2013

but TDServer.m is not available at all in 1.01.

It was renamed to TD_Server.m.

Making this #ifndef and code injection is much easier than transplanting TouchDB_vers.c generation from origin Xcode project file.

Sure, but you won't get an accurate version number. This is part of why I'm skeptical of CocoaPods; there is more to a project than just its source files. I don't think it's a wise idea to just dump all the source files of all the libraries you want to use into your Xcode project and expect them to build and run correctly.

snej commented on 1c726aa Jan 22, 2013

The change in TDReplicator is unnecessary. NSURL is immutable so -copy does the same thing as -retain, and there's already an implicit -retain call generated by ARC when assigning to _remote.

The change in TDRouter.m is wrong. The onFinished block needs to be called after the -stopNow call. Assigning _onFinished to a temporary variable keeps a reference to it, so it can still be called even though -stopNow released and zeroed the instance variable.

Owner

snej commented Jan 22, 2013

In general, it looks like you're building TouchDB in some unsupported way (through CocoaPods?) and probably haven't enabled ARC for all the source files. None of these refcounting changes should be necessary.

snej closed this Jan 22, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment