Group Group Group Group Group Group Group Group Group

Errata for Combine: Asynchronous Programming with Swift 1st Edition

I agree with “depending on the scenario”. If it was a fixed number of something small, as your example of “5 integers” suggested, I’d totally agree. I’d call that a “fixed cost” or “overhead” and not a “leak”.

As currently implemented, it’s an ever growing number of objects in memory that increases based on repeated execution of some code (triggered by user action in this case) and thus a memory leak. It is exactly a case of “memory is not released,” as you say. A retain cycle is one common cause of memory leaks, but certainly not the only one. “Failing to dispose of something that should be” is another. That’s a less common one now that we have ARC, but still possible, as this code demonstrates.

Now, I’ll be the first to agree that this is not very likely to be an issue in this app (unless it retains the whole publisher memory tree or something nasty like that). I mean, how many times is a user going to hit the add photos button (or save etc), realistically?

The reason I’m bothering to push on this, and care, is because this is likely to be an often-read book and teaching good memory management habits is important in that context. For a quick hack-test then there is no reason to care.

Imagine a scenario where someone wrote a photo processing app that processed a large batch of photos and re-used the PhotoWriter.swift file from this project. If they copied the code in actionSave()from MainViewController.swift which uses PhotoWriter to save an image and which follows the same pattern of saving the AnyCancellable into the subscriptions Set, then this memory leak is a lot more likely to be a significant problem for them (particularly if the number of photos processed is large).

Your idea of setting up the subscription once is interesting. Not sure how it works since the publisher goes away when the PhotosViewController is dismissed (presumably). Likewise for the other places where the AnyCancellable is saved into the subscriptions Set (PhotoWriter which uses a transitory Future publisher, etc).

Thanks again for the reply. Onward!

Chapter 5 (v1.0.2) - switchToLatest()

It is suggested that subscriptions are cancelled on switching to the next, and in the example (p211) we don’t bother completing publisher1 and 2 though we do complete publisher3. It lead me to believe that those publishers were cancelled, but we could happily switch back to the other publishers and get future values (but not the ones that were sent while attached to the other stream)

let publisherA = PassthroughSubject<Int, Never>()
let publisherB = PassthroughSubject<Int, Never>()
let publishers = PassthroughSubject<PassthroughSubject<Int, Never>, Never>()

publishers
    .switchToLatest()
    .sink(receiveCompletion: { print("Recieved completion", $0) },
          receiveValue: { print("Received value", $0) })

publisherA.send(0)
publishers.send(publisherA)
publisherA.send(1)
publishers.send(publisherB)
publisherA.send(2)
publisherB.send(3)
publishers.send(publisherA)
publisherA.send(4)

// Output is 1 3 4 
publisherA.send(completion: .finished)
publisherB.send(completion: .finished)
publishers.send(completion: .finished)

It isn’t hard to see how suggested corrections are slipping through the cracks. Perhaps, an errata that organized changes in an ordered list could be useful. It is difficult to find corrections in this forum.

That’s not really the case. We are following this forum and any additional feedback, but usually don’t fix individual feedback but accumulate it for additional releases. If time will allow, we’ll release additional releases in between but it’s not something we can guarantee at the moment.

Thanks ! And please keep submitting feedback.

Looking at the text I wouldn’t say that the book misleads the reader - it does say that storing subscriptions in subscriptions ties the subscription life to the view controller:

subscriptions is the collection where you will store any UI subscriptions tied to the lifecycle of the current view controller. When you bind your UI controls tying those subscriptions to the lifecycle of the current view controller is usually what you need. This way, in case the view controller is popped out of the navigation stack or dismissed otherwise, all UI subscriptions will be canceled right away.

I don’t really appreciate you reposting the code without the explanation what it does because this way you mislead folks who’ll read your blog. But that issue aside, if you want to handle your subscriptions differently (e.g. NOT tie their disposal to the disposal of your view controller like in the chapter) you’re welcome to do that - the approach you posted on your blog seems :+1:t3: to me.

Thanks for the clarification @icanzilb

@dad — I agree with Marin that your blog post is unfair/misleasinf because your definition of a memory leak is wrong - a leak would be when you accidentally leave some dead memory beyond the scope of another object in a way that cant be reclaimed. In here - We’re simply tying the subscription context to the view context which is a common and useful pattern.

In this specific case this is a “root” view controller which means these subscriptions will be accumulated, indeed, since the VC itself is never deallocated. In most common scenarios, though, these VCs are pushed and popped / presented and dismissed constantly, so tying these subscriptions to the view makes absolute sense.

Finally, don’t forget that book projects are not production apps. They are used to demonstrate specific points, ideas and concepts - and not an entire app with all of its production constraints.

Regardless, thank you again for the feedback! We might add a note about this in the next edition, but it’s definitely not a critical issue from our perspective as a book team and that behavior is planned in that specific context.

I can see your point - there is still a reference to memory and you could write code to dispose of the memory (hey! What do you know, that’s exactly what my blog post shows how to do!) and so by a strict definition of “memory leak”, sure, it’s not a “leak”. But it will be a memory leak if some new programmer copies the pattern in their root view controller (like in this simple example) and doesn’t understand that they are accumulating memory over time until their app crashes.

Unfortunately, the phrase “memory leak” seems to have triggered a bit of a defense reaction and overshadowed my point which was feedback that the book would be better if it pointed this issue out for less experienced programmers and taught readers a pattern to use in that case.

Because of the reaction my comments got here it seemed like I needed to write a blog post to point out the issue and provide at least one strategy myself - so I did.

In my blog post I specifically said,

“The problem with this pattern in this particular case, and other similar situations, is that the any AnyCancellable is never removed from the subscriptions Set and so it’s a memory leak. Sometimes this doesn’t matter - when the enclosing scope is going to go away and the code is only called once relatively soon before existing scope cleans up memory.

That all seems to be very specifically explaining what the issue is, when it’s an issue, and when it isn’t. Doesn’t seem “unfair/misleading” to me, but I’m sorry you feel that way; it wasn’t my intent that anyone should feel insulted or attacked.

In my blog post I made a point of linking to your book and saying “It’s a good book on Combine”, which I thought you would feel was of benefit to you; clearly you do not and so I have removed reference to your book from the post.

I have also heard your feedback and modified the language in the post to remove the phrase “memory leak”, since it seems to cause problems, and I tried to clarify the language in the section quoted above.

So, thanks for the feedback. Edits were made.

Chapter 8: Presenting a view controller as a future

The

.handleEvents(receiveCancel: {
  self.dismiss(animated: true)
})

part when displaying an alertController seems to be unnecessary as it will dismiss itself.

Chapter 3: Transforming operators

You create the Chatter object with the names of Charlotte and James, but they’re not really used in the rest of the code. The output is then showed as

Charlotte wrote: Hi, I'm Charlotte!
James wrote: Hi, I'm James!

but the following sink code will not produce that output:

chat
    .sink(receiveValue: { print($0.message.value) })»

Unless I’m missing something this is exactly the output the chapter has. If you run the final playground and remove the flatMap from the flatMap example, this is the output you’ll see:

——— Example of: flatMap ———
Hi, I'm Charlotte!
Hi, I'm James!
Hey guys, what are you up to?

What is exactly missing for you?

Good feedback, but I think it might be a misunderstanding

receiveCancel doesn’t mean the alert was cancelled, it means the subscription was cancelled. Consider the following scenario:

  1. Alert shows up
  2. Someone actively cancels the subscription (subscription.cancel or deallocating the owner)
  3. Alert is still visible because you didn’t do “cleanup”

If you do this sort of cleanup in the receiveCancel block, you won’t deal with this issue.,

Thanks again!

In Chapter 15’s sample code for HNReader, the timer does not fire when the VStack is after the ForEach loop:
ForEach(self.model.stories) { story in
VStack(alignment: .leading, spacing: 10) { …

However, when I switch those lines, the timer will fire normally:
VStack(alignment: .leading, spacing: 10) {
ForEach(self.model.stories) { story in

i.e. ForEach has to be inside the VStack for the timer to fire. I ran the sample code under Xcode 11.4.1

3 Likes

Hi @denisblondeau, thanks for reporting. We’re tracking this issue already, the behavior in SwiftUI has changed slightly since publishing the latest update of the book - we’ll fix it in the edition :+1:t3:

Ok - thank you for your reply ! :+1:

Nice! I spent over an hour on this yesterday trying to debug and track it down. Thanks for the post.

Yep - took me about that long to figure that one out ! This Combine book is really great - learned a lot !

I put all of my time into the TimeBadge and PostedBy helpers. I tried to make them observable and it didn’t work. Then I did a PRINT on the timer and noticed it canceled on the very next event.

Good catch!

2 Likes