Hi Jeremy, > - Log ----------------------------------------------------------------- > commit 6beba782f1bf951236813e0b46115b8102212c03 > Author: Jeremy Allison <[email protected]> > Date: Mon Apr 26 10:54:33 2010 -0700 > > Fix crash when rescheduling oplock open.
> + /*
> + * This is subtle. We must null out the callback
> + * before resheduling, else the first call to
> + * tevent_req_nterror() causes the _receive()
> + * function to be called, this causing tevent_req_post()
> + * to crash.
> + */
> + tevent_req_set_callback(smb2req->subreq, NULL, NULL);
> +
> im = tevent_create_immediate(smb2req);
> if (!im) {
> smbd_server_connection_terminate(smb2req->sconn,
I'm not sure this is correct.
I haven't looked in detail, but this looks like just hiding the real
problem.
The real problem is likely that we're abusing the tevent_req guidelines.
I think 8f67f873ace91964da066c421986e260aceba75b is maybe ok, for
getting stuff working, but I'd like to see the design changed.
smb2_deferred_open_timer() should not call smbd_smb2_request_dispatch().
The re-entrant should happen inside the smbd_smb2_create_* code,
the place were it decides to go async, instead of two layers above.
I think the smbd_smb2_create_* should setup a
smb2req->retry_callback(struct tevent_req *) function pointer.
smb2_deferred_open_timer() would then just call it should just call it.
I'd like to have something similar for smb1 (I know it would be a lot of
work), but the layer violation is really confusing.
The top level smb1/2 server code should not see any of this retry logic,
it should just do a foo_send() call set it's callback
on the returned tevent_req and get the final result with foo_recv().
All magic should be in one spot in the lower level.
metze
signature.asc
Description: OpenPGP digital signature
