VirtualBox

Changeset 60525 in vbox for trunk


Ignore:
Timestamp:
Apr 15, 2016 7:25:46 PM (9 years ago)
Author:
vboxsync
Message:

SrvIntNetR0.cpp: IntNetR0IfAbortWait w/ fNoMoreWaits=tru should never have pretended to be intnetR0IfDestruct! Changed the code to remember the fNoMoreWaits condition seperately from the intnetR0IfDestruct indicator, making the latter a RTTHREADNATIVE so we can restrict handle access in intnetR0IfRetainHandle() to just the destructor thread.

File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/VBox/Devices/Network/SrvIntNetR0.cpp

    r57846 r60525  
    218218     * This is shadowed by INTNETMACTABENTRY::fActive. */
    219219    bool                    fActive;
    220     /** Whether someone is currently in the destructor or has indicated that
    221      *  the end is nigh by means of IntNetR0IfAbortWait. */
    222     bool volatile           fDestroying;
     220    /** Whether someone has indicated that the end is nigh by means of IntNetR0IfAbortWait. */
     221    bool volatile           fNoMoreWaits;
    223222    /** The flags specified when opening this interface. */
    224223    uint32_t                fOpenFlags;
    … …  
    240239    RTSEMEVENT volatile     hRecvEvent;
    241240    /** Number of threads sleeping on the event semaphore. */
    242     uint32_t                cSleepers;
     241    uint32_t volatile       cSleepers;
    243242    /** The interface handle.
    244243     * When this is INTNET_HANDLE_INVALID a sleeper which is waking up
    245244     * should return with the appropriate error condition. */
    246245    INTNETIFHANDLE volatile hIf;
     246    /** The native handle of the destructor thread.  This is NIL_RTNATIVETHREAD when
     247     * the object is valid and set when intnetR0IfDestruct is in progress.  This is
     248     * used to cover an unlikely (impossible?)  race between SUPDRVSESSION cleanup
     249     * and lingering threads waiting for recv or similar. */
     250    RTNATIVETHREAD volatile hDestructorThread;
    247251    /** Pointer to the network this interface is connected to.
    248252     * This is protected by the INTNET::hMtxCreateOpenDestroy. */
    … …  
    771775DECLINLINE(int) intnetR0IfRetain(PINTNETIF pIf, PSUPDRVSESSION pSession)
    772776{
     777    Assert(((PINTNETIF)pIf->pvObj)->hDestructorThread == NIL_RTNATIVETHREAD);
     778
    773779    int rc = SUPR0ObjAddRefEx(pIf->pvObj, pSession, true /* fNoBlocking */);
    774780    AssertRCReturn(rc, rc);
     781
    775782    return VINF_SUCCESS;
    776783}
    … …  
    787794DECLINLINE(bool) intnetR0IfRelease(PINTNETIF pIf, PSUPDRVSESSION pSession)
    788795{
     796    Assert(((PINTNETIF)pIf->pvObj)->hDestructorThread == NIL_RTNATIVETHREAD);
     797
    789798    int rc = SUPR0ObjRelease(pIf->pvObj, pSession);
    790799    AssertRC(rc);
     800
    791801    return rc == VINF_OBJECT_DESTROYED;
    792802}
    … …  
    809819    NOREF(pvUser);
    810820    NOREF(hHandleTable);
    811     PINTNETIF pIf = (PINTNETIF)pvObj;
    812     if (pIf->hIf != INTNET_HANDLE_INVALID) /* Don't try retain it if called from intnetR0IfDestruct. */
     821
     822    PINTNETIF      pIf = (PINTNETIF)pvObj;
     823    RTNATIVETHREAD hDtorThrd;
     824    ASMAtomicUoReadHandle(&pIf->hDestructorThread, &hDtorThrd);
     825    if (hDtorThrd == NIL_RTNATIVETHREAD)
    813826        return intnetR0IfRetain(pIf, (PSUPDRVSESSION)pvCtx);
    814     return VINF_SUCCESS;
     827
     828    /* Allow intnetR0IfDestruct to call RTHandleTableFreeWithCtx to free
     829       the handle, but not even think about retaining a referenceas we don't
     830       want to confuse SUPDrv and risk having the destructor called twice. */
     831    if (hDtorThrd == RTThreadNativeSelf())
     832        return VINF_SUCCESS;
     833
     834    return VERR_SEM_DESTROYED;
    815835}
    816836
    … …  
    46184638    }
    46194639
    4620     const INTNETIFHANDLE    hIfSelf     = pIf->hIf;
    4621     const RTSEMEVENT        hRecvEvent  = pIf->hRecvEvent;
    4622     const bool              fDestroying = ASMAtomicReadBool(&pIf->fDestroying);
    4623     if (    hIfSelf    != hIf           /* paranoia */
    4624         ||  hRecvEvent == NIL_RTSEMEVENT
    4625         ||  fDestroying
    4626        )
    4627     {
     4640    const RTSEMEVENT hRecvEvent   = pIf->hRecvEvent;
     4641    const bool       fNoMoreWaits = ASMAtomicUoReadBool(&pIf->fNoMoreWaits);
     4642    RTNATIVETHREAD   hDtorThrd;
     4643    ASMAtomicReadHandle(&pIf->hDestructorThread, &hDtorThrd);
     4644    if (hDtorThrd != NIL_RTNATIVETHREAD)
     4645    {
     4646        /* See IntNetR0IfAbortWait for an explanation of hDestructorThread. */
    46284647        Log(("IntNetR0IfWait: returns VERR_SEM_DESTROYED\n"));
    46294648        return VERR_SEM_DESTROYED;
    46304649    }
    46314650
    4632     /*
    4633      * It is tempting to check if there is data to be read here,
    4634      * but the problem with such an approach is that it will cause
    4635      * one unnecessary supervisor->user->supervisor trip. There is
    4636      * already a slight risk for such, so no need to increase it.
    4637      */
    4638 
    4639     /*
    4640      * Increment the number of waiters before starting the wait.
    4641      * Upon wakeup we must assert reality, checking that we're not
    4642      * already destroyed or in the process of being destroyed. This
    4643      * code must be aligned with the waiting code in intnetR0IfDestruct.
    4644      */
    4645     ASMAtomicIncU32(&pIf->cSleepers);
    4646     int rc = RTSemEventWaitNoResume(hRecvEvent, cMillies);
    4647     if (pIf->hRecvEvent == hRecvEvent)
    4648     {
    4649         ASMAtomicDecU32(&pIf->cSleepers);
    4650         if (!pIf->fDestroying)
     4651    /* Check whether further waits have been barred by IntNetR0IfAbortWait. */
     4652    int rc;
     4653    if (   !fNoMoreWaits
     4654        && hRecvEvent != NIL_RTSEMEVENT)
     4655    {
     4656        /*
     4657         * It is tempting to check if there is data to be read here,
     4658         * but the problem with such an approach is that it will cause
     4659         * one unnecessary supervisor->user->supervisor trip. There is
     4660         * already a slight risk for such, so no need to increase it.
     4661         */
     4662
     4663        /*
     4664         * Increment the number of waiters before starting the wait.
     4665         * Upon wakeup we must assert reality, checking that we're not
     4666         * already destroyed or in the process of being destroyed. This
     4667         * code must be aligned with the waiting code in intnetR0IfDestruct.
     4668         */
     4669        ASMAtomicIncU32(&pIf->cSleepers);
     4670        rc = RTSemEventWaitNoResume(hRecvEvent, cMillies);
     4671        if (pIf->hRecvEvent == hRecvEvent)
    46514672        {
    4652             if (intnetR0IfRelease(pIf, pSession))
     4673            ASMAtomicDecU32(&pIf->cSleepers);
     4674            ASMAtomicReadHandle(&pIf->hDestructorThread, &hDtorThrd);
     4675            if (hDtorThrd == NIL_RTNATIVETHREAD)
     4676            {
     4677                if (intnetR0IfRelease(pIf, pSession))
     4678                    rc = VERR_SEM_DESTROYED;
     4679            }
     4680            else
    46534681                rc = VERR_SEM_DESTROYED;
    46544682        }
    … …  
    46574685    }
    46584686    else
     4687    {
    46594688        rc = VERR_SEM_DESTROYED;
     4689        intnetR0IfRelease(pIf, pSession);
     4690    }
     4691
    46604692    Log4(("IntNetR0IfWait: returns %Rrc\n", rc));
    46614693    return rc;
    … …  
    47044736    }
    47054737
    4706     const INTNETIFHANDLE    hIfSelf     = pIf->hIf;
    4707     const RTSEMEVENT        hRecvEvent  = pIf->hRecvEvent;
    4708     const bool              fDestroying = ASMAtomicReadBool(&pIf->fDestroying);
    4709     if (    hIfSelf    != hIf           /* paranoia */
    4710         ||  hRecvEvent == NIL_RTSEMEVENT
    4711         ||  fDestroying
    4712        )
    4713     {
     4738    const RTSEMEVENT hRecvEvent  = pIf->hRecvEvent;
     4739    RTNATIVETHREAD   hDtorThrd;
     4740    ASMAtomicReadHandle(&pIf->hDestructorThread, &hDtorThrd);
     4741    if (hDtorThrd != NIL_RTNATIVETHREAD)
     4742    {
     4743        /* This can only happen if we for some reason race SUPDRVSESSION cleanup,
     4744           i.e. the object count is set to zero without yet having removed it from
     4745           the object table, so we got a spurious "reference".  We must drop that
     4746           reference and let the destructor get on with its work.  (Not entirely sure
     4747           if this is practically possible on any of the platforms, i.e. whether it's
     4748           we can actually close a SUPDrv handle/descriptor with active threads still
     4749           in NtDeviceIoControlFile/ioctl, but better safe than sorry.) */
    47144750        Log(("IntNetR0IfAbortWait: returns VERR_SEM_DESTROYED\n"));
    47154751        return VERR_SEM_DESTROYED;
    47164752    }
    47174753
    4718     /*
    4719      * Set fDestroying if requested to do so and then wake up all the sleeping
    4720      * threads (usually just one).   We leave the semaphore in the signalled
    4721      * state so the next caller will return immediately.
    4722      */
    4723     if (fNoMoreWaits)
    4724         ASMAtomicWriteBool(&pIf->fDestroying, true);
    4725 
    4726     uint32_t cSleepers = ASMAtomicReadU32(&pIf->cSleepers) + 1;
    4727     while (cSleepers-- > 0)
    4728     {
    4729         int rc = RTSemEventSignal(pIf->hRecvEvent);
    4730         AssertRC(rc);
    4731     }
     4754    /* a bit of paranoia */
     4755    int rc = VINF_SUCCESS;
     4756    if (hRecvEvent != NIL_RTSEMEVENT)
     4757    {
     4758        /*
     4759         * Set fNoMoreWaits if requested to do so and then wake up all the sleeping
     4760         * threads (usually just one).   We leave the semaphore in the signalled
     4761         * state so the next caller will return immediately.
     4762         */
     4763        if (fNoMoreWaits)
     4764            ASMAtomicWriteBool(&pIf->fNoMoreWaits, true);
     4765
     4766        uint32_t cSleepers = ASMAtomicReadU32(&pIf->cSleepers) + 1;
     4767        while (cSleepers-- > 0)
     4768        {
     4769            int rc2 = RTSemEventSignal(pIf->hRecvEvent);
     4770            AssertRC(rc2);
     4771        }
     4772    }
     4773    else
     4774        rc = VERR_SEM_DESTROYED;
     4775
     4776    intnetR0IfRelease(pIf, pSession);
    47324777
    47334778    Log4(("IntNetR0IfWait: returns %Rrc\n", VINF_SUCCESS));
    … …  
    48314876
    48324877    /*
     4878     * For paranoid reasons we must now mark the interface as destroyed.
     4879     * This is so that any waiting threads can take evasive action (kind
     4880     * of theoretical case), and we can reject everyone else referencing
     4881     * the object via the handle table before we get around to removing it.
     4882     */
     4883    ASMAtomicWriteHandle(&pIf->hDestructorThread, RTThreadNativeSelf());
     4884
     4885    /*
    48334886     * We grab the INTNET create/open/destroy semaphore to make sure nobody is
    4834      * adding or removing interface while we're in here.  For paranoid reasons
    4835      * we also mark the interface as destroyed here so any waiting threads can
    4836      * take evasive action (theoretical case).
     4887     * adding or removing interfaces while we're in here.
    48374888     */
    48384889    RTSemMutexRequest(pIntNet->hMtxCreateOpenDestroy, RT_INDEFINITE_WAIT);
    4839     ASMAtomicWriteBool(&pIf->fDestroying, true);
    48404890
    48414891    /*
    … …  
    49214971
    49224972    /*
    4923      * Wakeup anyone waiting on this interface.
     4973     * Wakeup anyone waiting on this interface. (Kind of unlikely, but perhaps
     4974     * not quite impossible.)
    49244975     *
    49254976     * We *must* make sure they have woken up properly and realized
    … …  
    50455096    //pIf->fPromiscuousReal = false;
    50465097    //pIf->fActive          = false;
    5047     //pIf->fDestroying      = false;
     5098    //pIf->fNoMoreWaits     = false;
    50485099    pIf->fOpenFlags         = fFlags;
    50495100    //pIf->cYields          = 0;
    … …  
    50555106    //pIf->cSleepers        = 0;
    50565107    pIf->hIf                = INTNET_HANDLE_INVALID;
     5108    pIf->hDestructorThread  = NIL_RTNATIVETHREAD;
    50575109    pIf->pNetwork           = pNetwork;
    50585110    pIf->pSession           = pSession;
Note: See TracChangeset for help on using the changeset viewer.

© 2024 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette