
<!doctype html><html><head><title></title><style type="text/css">p.MsoNormal,p.MsoNoSpacing{margin:0}</style></head><body>How do you even know that a crash report is related to this issue? What I see is that this patch makes two assumptions,neither of them are documented nor backed in any way. But sure send it to the TC. Obviously, you will provide explanations what the assumptions underlying this patch are, how they are verified, and why no proper workaround can be made instead, so that the TC can make an informed decision, and you won't abuse your ties with Hugo to distort the vote, hmm...<br><br><div class="gmail_quote">Le 29 octobre 2019 11:48:16 GMT+02:00, Thomas Guillem <thomas@gllm.fr> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">

<div>On Tue, Oct 29, 2019, at 08:24, Rémi Denis-Courmont wrote:<br></div><blockquote type="cite" id="qt"><div>Did you investigate all crash reports to verify that none of them were not caused by the unspecified constraints that you claim exist?<br></div></blockquote><div><br></div><div>Not me personally, but iOS maintainers and Marvin forward to me every native crashes happening on iOS and macOS. Never saw one related to this specific issue.<br></div><div><br></div><blockquote type="cite" id="qt"><div>Did you check what the implementations on affected macOS versions actually does apply those constraints? All that I see is that the Darwin doc contradicts your statement.<br></div></blockquote><div><br></div><div>It won't be the first time that an Apple documentation contradict the actual behavior.<br></div><div><br></div><div>Every one but Rémi are in favor of pushing this patch. Should we vote on TC to resolve this conflict ?<br></div><div><br></div><blockquote type="cite" id="qt"><div><br></div><div class="qt-gmail_quote"><div>Le 29 octobre 2019 09:10:36 GMT+02:00, Thomas Guillem <thomas@gllm.fr> a écrit :<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:0pt;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><pre class="qt-k9mail"><div>OK, it's an UB, but on a constrained environment, that is macOS. For now, this UB seem to have 0 impact on users. As Marvin said, we had this issue since 7 years, VLC 3.x and 2.x users don't seem to be bothered by this.<br></div><div><br></div><div>We can't abort VLC for an UB that won't have any impact.<br></div><div><br></div><div>I'm OK with this patch and with a 3.0 backport. It is was I do locally every time I develop on macOS (in fact, I just define NDEBUG for this file).<br></div><div><br></div><div>One other argument in favor: this may be only temporary since Marvin finally reported this issue to Apple.<br></div><div><br></div><div><br></div><div>On Tue, Oct 29, 2019, at 04:26, Rémi Denis-Courmont wrote:<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(114, 159, 207);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>Le maanantaina 28. lokakuuta 2019, 22.35.49 EET Marvin Scholz a écrit :<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(173, 127, 168);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>On 28 Oct 2019, at 18:46, Rémi Denis-Courmont wrote:<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(138, 226, 52);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>Le maanantaina 28. lokakuuta 2019, 19.24.44 EET Marvin Scholz a écrit<br></div><div><br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(252, 175, 62);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>macOS pthread implementation for pthread_cond_wait is buggy, causing<br></div><div>sometimes the mutex to not be locked when running thread cancellation<br></div><div>cleanup handlers.<br></div><div>This causes random failures of vlc_mutex_unlock, as the cancellation<br></div><div>cleanup handler does not hold a lock on the mutex in rare cases.<br></div></blockquote><div>This won't work:<br></div><div><br></div><div>1) Darwin does not define unlocking a mutex by a thread that did not<br></div><div>lock it:<br></div><div>"Calling pthread_mutex_unlock with a mutex that the calling thread<br></div><div>does not<br></div><div>hold will result in undefined behavior." So you cannot simply assume<br></div><div>that it<br></div><div>will return an error and have no side effects; it probably only works<br></div><div>that way<br></div><div>on sunny day cases.<br></div><div><br></div><div>2) Even if Darwin did provide that guarantee, this would cause data<br></div><div>races if<br></div><div>the calling thread actually relied on the mutex being locked to access<br></div><div>lock-<br></div><div>protected data.<br></div></blockquote><div>Hi, thanks for the review.<br></div><div>I am aware of these issues, the intention of this is just to make debug<br></div><div>enabled<br></div><div>builds on macOS work again without randomly crashing all the time.<br></div></blockquote><div>No, you don't seem to be aware of what UB means. You cannot recover from UB. <br></div><div>Period. This is too late in code flow to put a workaround; we always put our <br></div><div>workarounds before the bug, not after it.<br></div><div><br></div><div>The only ways to handle UB is to 1) avoid getting there in the first place and <br></div><div>2) abort as simply and quickly as possible if it happens anyway. It's not even <br></div><div>about debugging, it's about security/safety.<br></div><div><br></div><div>If your argument is that the error actual happens, the correct solution is to <br></div><div>replace the assert with an abort. Otherwise you concretely might as well <br></div><div>deadlock, or fail the following lock or destroy, which are even worst than.<br></div><div><br></div><div>-- <br></div><div>レミ・デニ-クールモン<br></div><div><a href="http://www.remlab.net/">http://www.remlab.net/</a><hr>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></blockquote><div><hr>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></pre></blockquote></div><div><br></div><div>-- <br></div><div>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté. <br></div><div>_______________________________________________<br></div><div>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div>https://mailman.videolan.org/listinfo/vlc-devel<br></div></blockquote><div><br></div></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>