fix deadlock in message serialization using fragmented mode

[sneepy3]

A deadlock can occur during serialization in multithreaded mode using message fragmentation.

This fix is similar to #303.

[EugenioCollado]

Hi @sneepy3, thanks for raising this issue and for taking the time to contribute a fix.

I agree that the deadlock you described is real. However, I don’t think this change is safe in its current form. Releasing UXR_LOCK_STREAM_ID inside on_full_output_buffer_fragmented() means we lose exclusive ownership of the stream while fragmented serialization is still in progress. This could allow another thread to enter the same stream and interleave writes before the lock is acquired again.

There is also a lock-state issue in the failure path. If flush_callback() fails, or if no slot is freed, the function returns before reacquiring the mutex. However, the outer caller still assumes it owns the lock and will later call UXR_UNLOCK_STREAM_ID(). So this avoids the deadlock, but it introduces a race condition and a lock ownership bug.

I also don’t think the fix in 8acdff4 is an exact precedent here. In that case, the lock was released before continuing the fragmented prepare path, while still preserving a consistent lock state on return.

That said, the issue itself is worth fixing, so I am happy to keep discussing the right design here.

[sneepy3]

Hello @EugenioCollado, thank you very much for responding.

Regarding your first concern: to my understanding (please correct me if I'm wrong), the flush_callback is triggered once the buffer is full, so if another thread would try to start a stream, it should return an error or initiate a flush aswell instead of overwriting the buffer?

I totally agree with your second concern, but it could probably be addressed by locking again before returning.

[EugenioCollado]

Hi again @sneepy3. Once flush_callback() frees one or more reliable-history slots, another thread can lock the same stream and successfully start its own write before the original fragmented write reacquires the lock. In that case it can consume the newly freed slot(s) and advance stream->last_written, so the original message resumes in a stream state that may already contain another message in between its fragments. So the risk is interleaving / broken write atomicity, not direct buffer overwrite.

And yes, I agree that reacquiring the lock before returning would fix the second issue.

[sneepy3]

Thanks for the clarification. The only thing that I could come up with that does not significantly alter the current structure is wrapping the whole fragmented stream inside a session lock. Maybe you can come up with something better, as for my project, I can live with this solution.