Changeset 17306 in vbox

Timestamp:

Mar 3, 2009 6:23:42 PM (16 years ago)

Author:

vboxsync

Message:

VBoxNetFlt: Fixed missing transition to the 'destroying' state. Removed disabled code and unused hBlockEvent.

Location:

trunk/src/VBox/HostDrivers/VBoxNetFlt

Files:

• VBoxNetFlt.c (modified) (7 diffs)
• VBoxNetFltInternal.h (modified) (1 diff)

trunk/src/VBox/HostDrivers/VBoxNetFlt/VBoxNetFlt.c

@@ -646 +646 @@
      */
     rc = RTSemFastMutexRequest(pGlobals->hFastMtx); AssertRC(rc);
-#if 0 //#ifdef VBOXNETFLT_STATIC_CONFIG
-/** @todo r=bird: This looks kind of insane! I ASSUME this is specific to the
- * static config and to devices in the Unconnected state only. This *looks* like
- * a very unhealthy race between driver unloading and vboxNetFltFactoryCreateAndConnect.
- * If I'm right, then vboxNetFltFactoryCreateAndConnect should be made to back down one
- * way or the other, it should not the other way around. (see suggestion further down)
- *
- * If I'm wrong, then please explain in full.
- *
- * r=misha: this code is to prevent race conditions between PVBOXNETFLTINS construct (which occurs on binding to adapter
- * rather than on vboxNetFltFactoryCreateAndConnect for static_config) and destruction,
- * namely the instance returned by vboxNetFltFindInstanceLocked in vboxNetFltSearchCreateInstance could be actually the instance being removed.
- * I guess an approach similar to what you added to vboxNetFltFactoryCreateAndConnect could be used in vboxNetFltSearchCreateInstance in this case we could remove
- * this ugly hack.
- */
-    if (cRefs != 0)
-    {
-        Assert(cRefs < UINT32_MAX / 2);
-        RTSemFastMutexRelease(pGlobals->hFastMtx);
-        return false;
-    }
+#ifdef VBOXNETFLT_STATIC_CONFIG
     vboxNetFltSetState(pThis, kVBoxNetFltInsState_Destroying);
 #else
@@ -1018 +998 @@
     while (pCur)
     {
-#if 1
         uint32_t cRefs = ASMAtomicIncU32(&pCur->cRefs);
         if (cRefs > 1)
@@ -1087 +1066 @@
         AssertRCReturn(rc, rc);
 
-#else /* old code: */
-        VBOXNETFTLINSSTATE enmState = vboxNetFltGetState(pCur);
-        if (    enmState != kVBoxNetFltInsState_Destroying
-            &&  enmState != kVBoxNetFltInsState_Destroyed)
-        {
-            /** @todo r=bird: who is making sure this is a genuine reconnection case? */
-            vboxNetFltRetain(pCur, false /* fBusy */);
-            RTSemFastMutexRelease(pGlobals->hFastMtx);
-            *ppInstance = pCur;
-            return VINF_ALREADY_INITIALIZED;
-        }
-
-        /* wait 2 ms */ /** @todo r=bird: Doesn't RTThreadSleep() work here? If it's bust, I'd like to know it so we can fix it... */
-        RTSemFastMutexRelease(pGlobals->hFastMtx);
-        RTSemEventWait(pGlobals->hBlockEvent, 2);
-        rc = RTSemFastMutexRequest(pGlobals->hFastMtx);
-        AssertRCReturn(rc, rc);
-#endif
-
         /* try again */
         pCur = vboxNetFltFindInstanceLocked(pGlobals, pszName);
@@ -1159 +1119 @@
     {
 #ifdef VBOXNETFLT_STATIC_CONFIG
-# if 1 /** @todo r=bird: We need to fix the race here. The race is against release+destructor, the
-        * tell tale is a cRefs of and since cRefs is manipulated in an atomic fashion we can simply attempt
-        * to grab a reference atomically, the worst thing that can happen is that we have to decrement it again..
-        * Here is my suggestion: */
         /* Try grab a reference. If the count had already reached zero we're racing the
            destructor code and must back down. */
@@ -1189 +1145 @@
         if (pCur)
             vboxNetFltRelease(pCur, false /* fBusy */);
-
-# else
-        if (vboxNetFltGetState(pCur) == kVBoxNetFltInsState_Unconnected)
-        {
-            vboxNetFltRetain(pCur, false /* fBusy */); /** @todo r=bird: Who releases this on failure?  */
-            pCur->fDisablePromiscuous = !!(fFlags & INTNETTRUNKFACTORY_FLAG_NO_PROMISC);
-            rc = vboxNetFltConnectIt(pCur, pSwitchPort, ppIfPort);
-        }
-        else
-            rc = VERR_INTNET_FLT_IF_BUSY;
-        RTSemFastMutexRelease(pGlobals->hFastMtx);
-# endif
 #else
         rc = VERR_INTNET_FLT_IF_BUSY;
@@ -1403 +1347 @@
     RTSemFastMutexDestroy(pGlobals->hFastMtx);
     pGlobals->hFastMtx = NIL_RTSEMFASTMUTEX;
-
-#ifdef VBOXNETFLT_STATIC_CONFIG
-    RTSemEventDestroy(pGlobals->hBlockEvent);
-    pGlobals->hBlockEvent = NIL_RTSEMEVENT;
-#endif
-
 }
 
@@ -1427 +1365 @@
     if (RT_SUCCESS(rc))
     {
-#ifdef VBOXNETFLT_STATIC_CONFIG
-        rc = RTSemEventCreate(&pGlobals->hBlockEvent);
-        if (RT_SUCCESS(rc))
-        {
+        pGlobals->pInstanceHead = NULL;
+
+        pGlobals->TrunkFactory.pfnRelease = vboxNetFltFactoryRelease;
+        pGlobals->TrunkFactory.pfnCreateAndConnect = vboxNetFltFactoryCreateAndConnect;
+#if defined(RT_OS_WINDOWS) && defined(VBOX_TAPMINIPORT)
+        strcpy(pGlobals->SupDrvFactory.szName, "VBoxNetAdp");
+#else
+        strcpy(pGlobals->SupDrvFactory.szName, "VBoxNetFlt");
 #endif
-            pGlobals->pInstanceHead = NULL;
-
-            pGlobals->TrunkFactory.pfnRelease = vboxNetFltFactoryRelease;
-            pGlobals->TrunkFactory.pfnCreateAndConnect = vboxNetFltFactoryCreateAndConnect;
-#if defined(RT_OS_WINDOWS) && defined(VBOX_TAPMINIPORT)
-            strcpy(pGlobals->SupDrvFactory.szName, "VBoxNetAdp");
-#else
-            strcpy(pGlobals->SupDrvFactory.szName, "VBoxNetFlt");
-#endif
-            pGlobals->SupDrvFactory.pfnQueryFactoryInterface = vboxNetFltQueryFactoryInterface;
-            pGlobals->fIDCOpen = false;
-
-            return rc;
-#ifdef VBOXNETFLT_STATIC_CONFIG
-        }
-        RTSemFastMutexDestroy(pGlobals->hFastMtx);
-#endif
+        pGlobals->SupDrvFactory.pfnQueryFactoryInterface = vboxNetFltQueryFactoryInterface;
+        pGlobals->fIDCOpen = false;
+
+        return rc;
     }
 

trunk/src/VBox/HostDrivers/VBoxNetFlt/VBoxNetFltInternal.h

@@ -262 +262 @@
     /** The SUPDRV IDC handle (opaque struct). */
     SUPDRVIDCHANDLE SupDrvIDC;
-
-#ifdef VBOXNETFLT_STATIC_CONFIG
-    /** Something we can block on while waiting for an instance to be unlinked. */
-    RTSEMEVENT hBlockEvent;
-#endif
 } VBOXNETFLTGLOBALS;