Mike McCormack wrote:
ChangeLog:
- implement NtSignalAndWaitForSingleObject
Index: server/thread.c
RCS file: /home/wine/wine/server/thread.c,v retrieving revision 1.110 diff -u -p -r1.110 thread.c --- server/thread.c 4 Mar 2005 12:38:36 -0000 1.110 +++ server/thread.c 18 Apr 2005 16:28:02 -0000 @@ -577,6 +577,73 @@ void wake_up( struct object *obj, int ma } }
+/*
- signal_object
- Try signaling an event flag, a semaphore or a mutex.
- FIXME: find out which operation to do by checking the type of object
- */
+static void signal_object( obj_handle_t handle ) +{
- /* is it an event flag? */
- set_error( STATUS_SUCCESS );
- event_operation( handle, SET_EVENT );
- if (get_error() != STATUS_OBJECT_TYPE_MISMATCH)
return;- /* is it a semaphore? */
- set_error( STATUS_SUCCESS );
- release_semaphore( handle, 1, NULL );
- if (get_error() != STATUS_OBJECT_TYPE_MISMATCH)
return;- /* is it a mutex? */
- set_error( STATUS_SUCCESS );
- release_mutex( handle, NULL );
- if (get_error() != STATUS_OBJECT_TYPE_MISMATCH)
return;- set_error( STATUS_INVALID_HANDLE );
+}
I think this is a little ugly. I'd prefer to see this code do a switch on obj->ops and call the relevant function with the raw object instead of a handle.
+static void signal_and_wait( obj_handle_t hsignal, struct object *wait,
int flags, const abs_time_t *timeout )+{
- int r;
- if (!wait_on( 1, &wait, flags, timeout ))
return;- /* signal the object */
- signal_object( hsignal );
- if (get_error())
- {
end_wait( current );return;- }
- /* check if we woke ourselves up */
- if (!current->wait)
return;- r = check_wait( current );
- if (r != -1)
- {
end_wait( current );set_error( r );return;- }
- /* check if we need to wait */
- if (!(flags & SELECT_TIMEOUT))
return;- current->wait->user = add_timeout_user( ¤t->wait->timeout,
thread_timeout, current->wait );- if (!current->wait->user)
end_wait( current );+}
/* queue an async procedure call */ int thread_queue_apc( struct thread *thread, struct object *owner, void *func, enum apc_type type, int system, void *arg1, void *arg2, void *arg3 )
This seems to be pretty similar to select_on. Would it not be possible to use that function instead of duplicating that code?
Rob
I think this is a little ugly. I'd prefer to see this code do a switch on obj->ops and call the relevant function with the raw object instead of a handle.
I know it's ugly; I rewrote that bit three times, including once with an ops->signal() method. Implementing a object operation requires that the 3 operations are consistent. For example, the access flags required to perform must be consistent, so the get_handle_obj can be done only once. Unfortunately, semaphores and event flags require different permissions to execute ops->signal().
See conversation I had with Julliard below.
This seems to be pretty similar to select_on. Would it not be possible to use that function instead of duplicating that code?
I considered that. The problem is that we would have to signal the object in the middle of the setup for select_on(). That means passing new parameters to select_on(), and that function already looks a little busy.
Mike
<mike_m> julliard: i have a dilemma implementing SignalObjectAndWait <mike_m> the object to signal can be a semaphore, event flag, etc <mike_m> i get a handle <mike_m> so i have to resolve the object type by dereferencing the handle <mike_m> however, different objects require different permissions <mike_m> so a semaphore requires SEMAPHORE_MODIFY_STATE but a mutex does not <mike_m> so in the object->signal method, i'm going to pass an obj_handle_t instead of a struct object <mike_m> which is a little bit inconsistent <mike_m> is this still consistent with implementing an obj->ops->signal() ? <mike_m> esp. if it is obj->ops->signal( obj_handle_t ) ? <mike_m> or should i extract the handle's permissions and pass that to signal( struct object *obj, DWORD perms ) ? <mike_m> (you were right that this function is strangely designed) <julliard> mike_m: passing a handle to an object function is not good <mike_m> yeah. <julliard> i guess you'll need to do it by hand <mike_m> you mean without the function pointer <mike_m> without an obj->ops->signal function <julliard> yes
Mike McCormack wrote:
I think this is a little ugly. I'd prefer to see this code do a switch on obj->ops and call the relevant function with the raw object instead of a handle.
I know it's ugly; I rewrote that bit three times, including once with an ops->signal() method. Implementing a object operation requires that the 3 operations are consistent. For example, the access flags required to perform must be consistent, so the get_handle_obj can be done only once. Unfortunately, semaphores and event flags require different permissions to execute ops->signal().
Ok, I see the problem now. The only alternative I can think of is to try to open the object with the GENERIC_WRITE access right and implement generic mappings for each object (these mappings are really needed anyway for other situaitons).
This seems to be pretty similar to select_on. Would it not be possible to use that function instead of duplicating that code?
I considered that. The problem is that we would have to signal the object in the middle of the setup for select_on(). That means passing new parameters to select_on(), and that function already looks a little busy.
You could refactor the function so that the wait_on bit is separate. That seems to be the only thing you have to do before signalling the object.
Rob
Robert Shearman wrote:
Mike McCormack wrote:
I think this is a little ugly. I'd prefer to see this code do a switch on obj->ops and call the relevant function with the raw object instead of a handle.
I know it's ugly; I rewrote that bit three times, including once with an ops->signal() method. Implementing a object operation requires that the 3 operations are consistent. For example, the access flags required to perform must be consistent, so the get_handle_obj can be done only once. Unfortunately, semaphores and event flags require different permissions to execute ops->signal().
Ok, I see the problem now. The only alternative I can think of is to try to open the object with the GENERIC_WRITE access right and implement generic mappings for each object (these mappings are really needed anyway for other situaitons).
Actually, I just thought about one more solution. You could open the object with using an access right of 0, meaning that anything is accepted, then check the access right manually. Still a bit of a kludge, but I think it is better than the brute force chaining.
Rob
Actually, I just thought about one more solution. You could open the object with using an access right of 0, meaning that anything is accepted, then check the access right manually. Still a bit of a kludge, but I think it is better than the brute force chaining.
Yeah, I'm not really happy with it the way it is either, thus the FIXME. For now, it works and demonstrates the problem with SignalObjectAndWait... I'm interested to hear Alexandre's opinion before trying to fix it myself.
Mike
-
Mike McCormack -
Robert Shearman