fix ambed openstream timeout issue #222
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch fixes an old bug on xlxd related with ambed openstream timeout, actually if the openstream response packet from ambed to xlxd is completely lost then no problem happens at all, however if timeout is reached but xlxd still gets the packet after timeout then transcoding gets fully broken with crashing streams forever until xlxd is restarted.
The problem is discussed at issues #115 , #141 and probably also #168 , the workaround of @lucamarche-iz1mlt is to tweak this timeout (and inherently some other timeouts) to an high value to avoid it but the bug is still there... also it should be easy for a malicious user to take advantage of this to break any XLX, as easy as simulate an openstream response packet...
The problem happens because when xlxd gets openstream response packet it will always call m_SemaphoreOpenStream.Notify(), thus incrementing semaphore m_Count that will never be decremented because WaitFor() already timedout... and... this incremented m_Count will cause all subsequent WaitFor() calls to return immediately, before getting the required stream details... My solution is maybe not the most elegant code but it fixes the problem, I didn't use just Reset() as it could eventually discard concurrent notifies...