Freeze after that an exception is raised during initialization (other sources never shutdown)

Romain Beauxis added a comment - 17/May/11 4:41 PM I'll fix that before 1.0..

David Baelde added a comment - 19/May/11 7:25 PM It would be useful to not just quote the ML, but process vague descriptions into something easily reproducible. I may be skimming too fast through my backlog but I couldn't understand that bug... it doesn't matter since Romain seems to understand the issue.

MartinS4 added a comment - 27/May/11 10:13 AM - edited @David: this issue happen in dynamic context, every time you add a dynamic connection to a closed (icecast/shoutcast) port. That's it ! I can add the following details: while the liquidsoap process CAN'T be killed or breaked (Ctrl-C) in a normal way, telnet interactions are still possible but you can't remove any connection at all. However, variable that is used in the liq script ( for example to dyn_sources []), which hold uris, sources and outputs) is updated correctly.

Romain Beauxis added a comment - 09/Jun/11 10:15 PM Here's my understanding on this one: when an exception is raised upon creatinh a new output, although it would have crashed the whole liquidsoap prior to dynamic source creation, the exception, when raised during a dynamic creation of an output actually freezes the main liquidsoap thread...

Romain Beauxis added a comment - 14/Jun/11 8:08 PM Hi all! I cant seem to reproduce with the lastest hg code ( default@5e00569d1839 :20110614:145842) Martin, could you add a minimal example to reproduce the issue to the this ticket? Thanks!

MartinS4 added a comment - 23/Jun/11 7:24 AM Hi Romain, Of course I can :) This is all in a e-mail i've sent to you and the list on 10/06 under the "[Savonet-users] robinet alpha is available !" subject. Here it is: ===== Okay, I dig a little into this issue this morning, and to make this clear, here is the simple test you could run to reproduce: audio = input.alsa(device="pulse") output.shoutcast(%mp3, host="ns0.proxad.net", port = 8010, password = "anything", audio) returns: init: exception encountered during main phase:   Tutils.Exit exception: Tutils.Exit Raised by primitive operation at file "", line 0, characters 0-0 Note: the process is stuck as well. (kill -9) audio = input.alsa(device="pulse") output.shoutcast(%mp3, host="yahoo.fr", port = 8010, password = "anything", audio) returns nothing at first ! looks like the process is stuck. Then the previous exception is raised, after ~3 min. Btw the process remains unbreakable. (kill -9) of course I've put ns0.proxad.net and yahoo.fr to show evidence. And now look what is interesting ! telnet yahoo.fr 8010 Trying 87.248.120.148... (nothing happen => ^C) telnet ns0.proxad.net 8010 Trying 212.27.32.2... telnet: Unable to connect to remote host: Connection refused The connection mecanism is not handled the same way by remote server/firewall. I would guess that yahoo "drops" and proxad "rejects". Anyway it doesn't change the process state in the end but it could be a path. So for me it has nothing to do with any shoutcast / icecast thing. It's pure socket-related. =====

Romain Beauxis added a comment - 24/Jun/11 4:06 PM Ok, I think this a bug related to input.alsa. I have renamed the title accordingly and will work on this for the final 1.0 release.

David Baelde added a comment - 26/Jun/11 12:15 AM We may need to split this ticket in several. But there's clearly an issue that is independent of ALSA: following instructions in an old mail from Martin, I tried output.shoutcast(%mp3, host="yahoo.fr", port = 8010, password = "anything", blank()) And it just freezes. According to Martin it'll issue an exception after 3 minutes. If you attempt to kill liquidsoap, it'll wait for the frozen thread, which takes forever. However, I did not manage (yet?) to freeze streaming by dynamically creating the broken output. To me the first problem is not the exception, but the fact that it takes 3 minutes to timeout.

MartinS4 added a comment - 01/Aug/11 1:04 PM - edited The behavior for "connection refused" has changed in mercurial these days. Now it just exit (which is not expected neither :D) 2011/08/01 14:58:34 [4191d049-88fc-46ee-ac7f-200562b323a0:2] Connection failed: could not connect to host: Connection refused in connect()! 2011/08/01 14:58:34 [threads:3] Thread "wallclock_pulse" exited. 2011/08/01 14:58:34 [main:3] Shutdown started! 2011/08/01 14:58:34 [main:3] Waiting for threads to terminate... 2011/08/01 14:58:34 [dummy:4] Activations changed: static=[], dynamic=[]. 2011/08/01 14:58:34 [source:4] Source dummy gets down. 2011/08/01 14:58:34 [blank_5432:4] Activations changed: static=[], dynamic=[]. 2011/08/01 14:58:34 [source:4] Source blank_5432 gets down. 2011/08/01 14:58:34 [clock.wallclock_main:3] Streaming loop stopped. 2011/08/01 14:58:34 [threads:3] Thread "wallclock_main" terminated (0 remaining). 2011/08/01 14:58:34 [main:3] Cleaning downloaded files... 2011/08/01 14:58:34 >>> LOG END To reproduce: $ liquidsoap 'output.shoutcast(%mp3, host="ns0.proxad.net", port = 8010, password = "anything", blank())' When on the command line, it could make sense to exit... In a server/dynamic context this is not really relevant

Romain Beauxis added a comment - 01/Aug/11 2:24 PM Well, it should be expected by defaulr (restart=false).. What happens if you set restart to true in output.shoutcast's parameters?

Romain Beauxis added a comment - 02/Aug/11 12:47 AM Ok, I have done some investigation on this issue. It appears that this is a more complex instance of LS-503 : when output.icecast fails, the fix for LS-503 added a proper termination of output.icecast. However, it did not address the termination of other sources owned by the clock.. I have committed a LS-527 branch with a proposed fix. As it is not my department, I cannot really be sure it is fit for the next beta release.. David, if you have a minute to review it tomorrow, I'll increasing your beer counter for next month (which is already at one ;-) )

David Baelde added a comment - 02/Aug/11 9:46 AM I'll try to review the proposed patch tonight. I have had time only for a quick look for now. In the meantime, a more detailed explanation would help. From what I gathered, the fix only addresses cases where the clock which fails to initialize is owned by a source, for example an ALSA input. I can image that there is something special going on in such cases, perhaps it deserves some special code. However, as I pointed out in an earlier comment, I could reproduce a similar bug without ALSA, thus without a source-owned clock. There may be something more general going on... We'll need to check whether the fix solves the more general problem, and why. If not, see if we ship this with the next beta even if we don't have the more general solution.

Romain Beauxis added a comment - 02/Aug/11 1:05 PM Hey, Thanks for the heads up. I've been thinking about the patch and it is incorrect as it is. What needs to happen -- and should be reasonable I think, is that once a source fails to initialize during the startup phase, all source that have been woken up before should be put to sleep. As it is now, the patch puts to sleep all sources, regardless of their activation. Would such a change seem sensible to you? Btw, one change for the long run we could think of is having at least self#sleep to be idempotent. That usually simplifies the code a lot...

Romain Beauxis added a comment - 03/Aug/11 3:36 AM Ok, I have a much better patch which is attached. Basically, I added a Utils.process_with_rollback function which takes care to rolling back all previously processed elements before raising an exception.. The patch looks fine to me now..

Romain Beauxis added a comment - 03/Aug/11 3:37 AM Proposed fix

David Baelde added a comment - 03/Aug/11 1:01 PM Hi Romain. I'll be leaving in a few hours, and I didn't have time to review changes or propose a solution. I'm sorry. If you're convinced about your patch, go ahead. But I have a feeling that a quick fix won't be sufficient (note that the code is already full of rollbacks)... Perhaps this issue is not too critical: most users don't use dynamic outputs, and will thus only experience the freeze in case of error, which is not as bad as a freeze occurring when liquidsoap should keep running.

Romain Beauxis added a comment - 04/Aug/11 3:41 AM Thanks for taking the time to respond! Reading from the beginning, I think that there are two issues mentioned here:  * Too long timeout on output.icecast  * A freeze when raising an issue in output.icecast while using input.alsa under it Concerning the first one, I believe that the recent come-back of timeout in output.icecast should allow to fix the first issue. It could be possible, though, that this is a timeout during Unix.connect, which is not covered (yet?). In any case, any issue on that would be better in a frest report I think. As for the second one, I believe that my patch is at least reasonable. As I said, the only difference is that with the patch, we no longer call #leave on the first source that fails but also on all sources that were initialized before. I think it should have been done before anyway and should not hurt the cases where it does not happen, thus the majority of the cases. The code is indeed full of rollbacks but, unfortunately, I am not sure that there is an elegant way to factorize it because one type of rollback happens without an exception and the other one with one... So I think I'm gonna go ahead and merge the changes. After that, I'll just resolve the ticket and we'll use the close functionality for a good reason if nothing bad happens.. :-)

Romain Beauxis added a comment - 04/Aug/11 4:03 AM Proposed fix pushed to default in 6a0dbd421336. Branch is still open and should be closed when we set this to "closed"

David Baelde added a comment - 24/Aug/11 8:10 AM Just wanted to mention a possible link with LS-532

David Baelde added a comment - 28/Sep/11 9:02 AM Romain's solution has been merged a while ago, and I strengthened it as part of LS-532 in b63faf981ac2.