Check if stream is writable to avoid infinite loop in write_content_chunked while using SSE

[aldoshkind]

Hi @yhirose ! Here is PR which fixes issue described in #1475

As you asked here is the copy of issue description:

I am using server-sent events and faced infinite loop. In my case infinite loop occurs when I reopen web-page/web-browser several times. How to reproduce: run SSE example ssesvr.cc, open browser and reload page several times (5 or 6 in my case on 8-core processor). Voila, page not loading! Infinite loop is here: https://github.com/yhirose/cpp-httplib/blob/master/httplib.h#L3764 My hotfix looks like this: while (data_available && !is_shutting_down() && strm.is_writable()) { (add && strm.is_writable()). Not sure it's absolutely correct, so no PR from me :) Also I see there are similar pieces of code, maybe they are affected too. Please investigate the problem! Thanks!

[yhirose]

@aldoshkind thanks for the pull request. The change looks good to me. Could you do the same at L3692 as well?

[aldoshkind]

I added same check on lines L3692 and L3646 and tested all three modes manually. I got new questions while reading the code:

• shouldn't we return false if stream isn't writable anymore?
• shouldn't we set error in write_content_without_length like in write_content_chunked and write_content?
• if so, which error should we return? What does Error::Connection mean? It would be awesome to have comments on all the errors :)

[yhirose]

@aldoshkind thanks for the changes and thoughtful questions. When I looked at your first question with your changes, it turns out that DataSink::is_writable() method isn't necessary and the 'writable' check should be done in DataSink::write() with strm.is_writable(). By doing this, if the write stream becomes unable, the client returns Error::Write.

Regarding your second question, write_content_without_length doesn't need error, since it's only used in Server. Error is only available for Client.

I confirmed that httplib.h in #1483 fixes the SSE problems on my MacBook. When you have time could you test the version on your machine? Thanks a lot!

[aldoshkind]

@yhirose I checked #1483 , looks like everything works nice, thanks! :)

Nevertheless, I think checking strm.is_writable() in write_content_chunked and its friends is still good idea as it will prevent infinite loop even if user returns from content_provider callback without calling DataSink::write() for some reason. Kind of additional protection level :)

[yhirose]

@aldoshkind you are right. I made the change. Thanks for your fine contribution and suggestion!

[aldoshkind]

@yhirose Thank you for rapid response and awesome lib :)