Skip to content

Draft: mf: Prevent double submission of session commands.

Yuxuan Shui requested to merge yshui/wine:mf-session-multiple-submission into master

This is not the right fix, and I need some help. Let me explain.

session_submit_command checks that:

  1. session->commands is empty, and
  2. SESSION_FLAG_PENDING_COMMAND is NOT set

before submitting the session_op. The assumption is, if there are queued commands, or if there is a command currently running, that pending command will submit subsequent commands when it completes.

However, in this call chain:

#0  0x00006fffe1d27ad7 in create_async_result (object=object@entry=0x0, callback=callback@entry=0x7e62489f1c08,
    state=state@entry=0x7e61cb5322b0, out=out@entry=0x7e6242bac7a0) at ../dlls/rtworkq/queue.c:1229
#1  0x00006fffe1d222d5 in RtwqCreateAsyncResult (object=object@entry=0x0, callback=callback@entry=0x7e62489f1c08,
    state=state@entry=0x7e61cb5322b0, out=out@entry=0x7e6242bac7a0) at ../dlls/rtworkq/queue.c:1245
#2  0x00006fffd45e2096 in MFPutWorkItem (queue=queue@entry=1, callback=callback@entry=0x7e62489f1c08,
    state=state@entry=0x7e61cb5322b0) at ../dlls/mfplat/queue.c:46
#3  0x00006fffe41244ee in session_command_complete (session=0x7e62489f1be0) at ../dlls/mf/session.c:975
#4  0x00006fffe412d00f in session_reset (session=session@entry=0x7e62489f1be0) at ../dlls/mf/session.c:1051
#5  0x00006fffe41299fc in session_handle_source_shutdown (session=session@entry=0x7e62489f1be0)
    at ../dlls/mf/session.c:2957
#6  0x00006fffe4126e90 in session_events_callback_Invoke (iface=0x7e62489f1c20, result=<optimized out>)
    at ../dlls/mf/session.c:4445

nowhere was the SESSION_FLAG_PENDING_COMMAND flag set. In addition to that, session_events_callback_Invoke is not a command. So this could happend:

Thread 1                                 | Thread 2
call session_events_callback_Invoke      |
                                         | call session_submit_command
                                         | <critical section>
                                         | list_empty(&session->commands) is true, and PENDING_COMMAND not set
                                         | MFPutWorkItem(op), and op->submitted = TRUE. <- 1.
                                         | list_add(&session->commands, op) <- 2.
                                         | </critical_section>
call session_handle_source_shutdown      |
<critical_section>                       |
call session_reset                       |
call session_purge_pending_commands      |
op not purged because it's submitted (1) |
call session_command_complete            |
list_head(&session->commands)            |
op is returned because of (2)            |
MFPutWorkItem(op) again                  |
</critical_section>                      |

The result is this op will be executed twice, and also double freed, because session_commands_callback_Invoke releases the op.


There is even another way an op can be submitted twice, session_handle_start_error calls session_reset, which already calls session_command_complete, and later it calls session_command_complete_with_event again. I think a missing return?

Edited by Yuxuan Shui

Merge request reports

Loading