Re: [PATCH 0/1] MR5789: server: Remove limitation for waiting on idle_event of the current process.

[Dmitry Timoshkov (＠dmitry)]

On Tue Jun  4 17:31:19 2024 +0000, Rémi Bernon wrote:
> I think that setting the event in `msg_queue_add_queue` is dangerous. It
> just works by chance because the queue is always last in the wait
> handles, but it could simply break any time if `wait_on` accesses the
> wait object again after all `obj->ops->add_queue` calls.
> What about making `wait_on` aware of idlenes and moving the set_event
> out of there? Like that:
> diff --git a/server/queue.c b/server/queue.c
> index e881e40271d..5a077e383e3 100644
> --- a/server/queue.c
> +++ b/server/queue.c
> @@ -1098,7 +1098,6 @@ static int is_queue_hung( struct msg_queue *queue )
>  static int msg_queue_add_queue( struct object *obj, struct
> wait_queue_entry *entry )
>  {
>      struct msg_queue *queue = (struct msg_queue *)obj;
> -    struct process *process = get_wait_queue_thread(entry)->process;
>
>      /* a thread can only wait on its own queue */
>      if (get_wait_queue_thread(entry)->queue != queue)
> @@ -1106,7 +1105,6 @@ static int msg_queue_add_queue( struct object
> *obj, struct wait_queue_entry *ent
>          set_error( STATUS_ACCESS_DENIED );
>          return 0;
>      }
> -    if (process->idle_event && !(queue->wake_mask & QS_SMRESULT))
> set_event( process->idle_event );
>
>      if (queue->fd && list_empty( &obj->wait_queue ))  /* first on the
> queue */
>          set_fd_events( queue->fd, POLLIN );
> @@ -1255,6 +1253,13 @@ static int check_queue_input_window( struct
> msg_queue *queue, user_handle_t wind
>      return ret;
>  }
>
> +void set_thread_idle( struct thread *thread )
> +{
> +    struct msg_queue *queue = thread->queue;
> +    if ((queue->wake_mask & QS_SMRESULT)) return;
> +    if (thread->process->idle_event) set_event(
> thread->process->idle_event );
> +}
> +
>  /* make sure the specified thread has a queue */
>  int init_thread_queue( struct thread *thread )
>  {
> diff --git a/server/thread.c b/server/thread.c
> index 55bd63d3030..8447d82f0a5 100644
> --- a/server/thread.c
> +++ b/server/thread.c
> @@ -778,7 +778,7 @@ static int wait_on( const select_op_t *select_op,
> unsigned int count, struct obj
>  {
>      struct thread_wait *wait;
>      struct wait_queue_entry *entry;
> -    unsigned int i;
> +    unsigned int i, idle = 0;
>
>      if (!(wait = mem_alloc( FIELD_OFFSET(struct thread_wait,
> queues[count]) ))) return 0;
>      wait->next    = current->wait;
> @@ -802,8 +802,12 @@ static int wait_on( const select_op_t *select_op,
> unsigned int count, struct obj
>              end_wait( current, get_error() );
>              return 0;
>          }
> +
> +        if (obj == (struct object *)current->queue) idle = 1;
>      }
> -    return 1;
> +
> +    if (idle) set_thread_idle( current );
> +    return current->wait ? 1 : 0;
>  }
>
>  static int wait_on_handles( const select_op_t *select_op, unsigned int
> count, const obj_handle_t *handles,
> diff --git a/server/user.h b/server/user.h
> index 1cd42506c23..afe9b807836 100644
> --- a/server/user.h
> +++ b/server/user.h
> @@ -112,6 +112,7 @@ extern void set_queue_hooks( struct thread *thread,
> struct hook_table *hooks );
>  extern void inc_queue_paint_count( struct thread *thread, int incr );
>  extern void queue_cleanup_window( struct thread *thread, user_handle_t
> win );
>  extern int init_thread_queue( struct thread *thread );
> +extern void set_thread_idle( struct thread *thread );
>  extern int attach_thread_input( struct thread *thread_from, struct
> thread *thread_to );
>  extern void detach_thread_input( struct thread *thread_from );
>  extern void set_clip_rectangle( struct desktop *desktop, const
> rectangle_t *rect,

This doesn't belong to this patch and probably should be sent as a separate change.