Changeset 31195 in vbox

Timestamp:

Jul 29, 2010 8:40:17 AM (15 years ago)

Author:

vboxsync

Message:

Attempt to fix crashes in DevVGA::vgaPortTakeScreenshot (xTracker 5146).

Location:

trunk/src/VBox

Files:

• Devices/Graphics/DevVGA.cpp (modified) (14 diffs)
• Main/DisplayImpl.cpp (modified) (2 diffs)

trunk/src/VBox/Devices/Graphics/DevVGA.cpp

@@ -1763 +1763 @@
 static void vga_draw_text(VGAState *s, int full_update)
 #else
-static int vga_draw_text(VGAState *s, int full_update)
+static int vga_draw_text(VGAState *s, int full_update, bool fFailOnResize)
 #endif /* !VBOX */
 {
@@ -1851 +1851 @@
     if (width != (int)s->last_width || height != (int)s->last_height ||
         cw != s->last_cw || cheight != s->last_ch) {
+#ifdef VBOX
+        if (fFailOnResize)
+        {
+            /* The caller does not want to call the pfnResize. */
+            return VERR_TRY_AGAIN;
+        }
+#endif /* VBOX */
         s->last_scr_width = width * cw;
         s->last_scr_height = height * cheight;
@@ -2175 +2182 @@
 static void vga_draw_graphic(VGAState *s, int full_update)
 #else
-static int vga_draw_graphic(VGAState *s, int full_update)
+static int vga_draw_graphic(VGAState *s, int full_update, bool fFailOnResize)
 #endif /* !VBOX */
 {
@@ -2262 +2269 @@
         ||  offsets_changed)
     {
+        if (fFailOnResize)
+        {
+            /* The caller does not want to call the pfnResize. */
+            return VERR_TRY_AGAIN;
+        }
         int rc = vga_resize_graphic(s, disp_width, height, v);
         if (rc != VINF_SUCCESS)  /* Return any rc, particularly VINF_VGA_RESIZE_IN_PROGRESS, to the caller. */
@@ -2468 +2480 @@
     VGAState *s = vga_state;
 #else /* VBOX */
-static int vga_update_display(PVGASTATE s, bool fUpdateAll)
+static int vga_update_display(PVGASTATE s, bool fUpdateAll, bool fFailOnResize)
 {
     int rc = VINF_SUCCESS;
@@ -2503 +2515 @@
 #ifdef VBOX
         if (fUpdateAll) {
-            /* A full update is requested. Special processing for a "blank" mode is required. */
+            /* A full update is requested. Special processing for a "blank" mode is required, because
+             * the request must process all pending resolution changes.
+             *
+             * Appropriate vga_draw_graphic or vga_draw_text function, which checks the resolution change,
+             * must be called even if the screen has been blanked, but then the function should do no actual
+             * screen update. To do this, pfnUpdateRect is replaced with a nop.
+             */
             typedef DECLCALLBACK(void) FNUPDATERECT(PPDMIDISPLAYCONNECTOR pInterface, uint32_t x, uint32_t y, uint32_t cx, uint32_t cy);
             typedef FNUPDATERECT *PFNUPDATERECT;
@@ -2524 +2542 @@
 
             /* Do a complete redraw, which will pick up a new screen resolution. */
-            if (fBlank) {
-                s->graphic_mode = GMODE_BLANK;
-                vga_draw_blank(s, 1);
-            } else if (s->gr[6] & 1) {
+            if (s->gr[6] & 1) {
                 s->graphic_mode = GMODE_GRAPH;
-                rc = vga_draw_graphic(s, 1);
+                rc = vga_draw_graphic(s, 1, false);
             } else {
                 s->graphic_mode = GMODE_TEXT;
-                rc = vga_draw_text(s, 1);
+                rc = vga_draw_text(s, 1, false);
             }
 
@@ -2561 +2576 @@
             rc =
 #endif /* VBOX */
-            vga_draw_text(s, full_update);
+            vga_draw_text(s, full_update, fFailOnResize);
             break;
         case GMODE_GRAPH:
@@ -2567 +2582 @@
             rc =
 #endif /* VBOX */
-            vga_draw_graphic(s, full_update);
+            vga_draw_graphic(s, full_update, fFailOnResize);
             break;
         case GMODE_BLANK:
@@ -3480 +3495 @@
     {
 #ifdef DEBUG_sunlover
-        AssertMsgFailed(("vgaR3IOPortHGSMIRead: Port=%#x cb=%d\n", Port, cb));
+        Log(("vgaR3IOPortHGSMIRead: Port=%#x cb=%d\n", Port, cb));
 #endif
         rc = VERR_IOM_IOPORT_UNUSED;
@@ -4861 +4876 @@
     }
 
-    rc = vga_update_display(pThis, false);
+    rc = vga_update_display(pThis, false, false);
     if (rc != VINF_SUCCESS)
     {
@@ -4889 +4904 @@
     pThis->graphic_mode = -1; /* force full update */
 
-    return vga_update_display(pThis, true);
+    return vga_update_display(pThis, true, false);
 }
 
@@ -4978 +4993 @@
 
     /*
-     * Do a complete screen update first to resolve any pending resize issues.
+     * Get screenshot. This function will fail if a resize is required.
+     * So there is not need to do a 'updateDisplayAll' before taking screenshot.
      */
-    updateDisplayAll(pThis);
 
     /*
@@ -5028 +5043 @@
             /* Make the screenshot.
              *
-             * The second parameter is 'false' because the current display state, already updated by the
-             * pfnUpdateDisplayAll call above, is being rendered to an external buffer using a fake connector.
-             * That is if display is blanked, we expect a black screen in the external buffer.
+             * The second parameter is 'false' because the current display state is being rendered to an
+             * external buffer using a fake connector. That is if display is blanked, we expect a black
+             * screen in the external buffer.
+             * If there is a pending resize, the function will fail.
              */
-            rc = vga_update_display(pThis, false);
+            rc = vga_update_display(pThis, false, true);
 
             /* Restore. */

trunk/src/VBox/Main/DisplayImpl.cpp

@@ -2433 +2433 @@
     uint32_t cx = 0;
     uint32_t cy = 0;
+    int vrc = VINF_SUCCESS;
 
 #ifdef VBOX_WITH_OLD_VBVA_LOCK
-    int vrc = VMR3ReqCallWait(pVM, VMCPUID_ANY, (PFNRT)Display::displayTakeScreenshotEMT, 6,
+    int cRetries = 5;
+
+    while (cRetries-- > 0)
+    {
+        vrc = VMR3ReqCallWait(pVM, VMCPUID_ANY, (PFNRT)Display::displayTakeScreenshotEMT, 6,
                               pDisplay, aScreenId, &pu8Data, &cbData, &cx, &cy);
+        if (vrc != VERR_TRY_AGAIN)
+        {
+            break;
+        }
+
+        RTThreadSleep(10);
+    }
 #else
     /* @todo pfnTakeScreenshot is probably callable from any thread, because it uses the VGA device lock. */
-    int vrc = VMR3ReqCallWait(pVM, VMCPUID_ANY, (PFNRT)pDrv->pUpPort->pfnTakeScreenshot, 5,
+    vrc = VMR3ReqCallWait(pVM, VMCPUID_ANY, (PFNRT)pDrv->pUpPort->pfnTakeScreenshot, 5,
                               pDrv->pUpPort, &pu8Data, &cbData, &cx, &cy);
 #endif /* !VBOX_WITH_OLD_VBVA_LOCK */
@@ -2525 +2537 @@
         rc = setError(E_NOTIMPL,
                       tr("This feature is not implemented"));
+    else if (vrc == VERR_TRY_AGAIN)
+        rc = setError(E_UNEXPECTED,
+                      tr("This feature is not available at this time"));
     else if (RT_FAILURE(vrc))
         rc = setError(VBOX_E_IPRT_ERROR,