Uploaded image for project: 'Core ReactOS'
  1. Core ReactOS
  2. CORE-20264

VirtualBox 4.1.0 - 4.3.12 versions still don't load/save VM configuration from XML configs properly

    XMLWordPrintable

Details

    • Bug
    • Resolution: Unresolved
    • Major
    • None
    • Wine
    • None
    • ReactOS 0.4.16-dev-1402-gf7c36c6 in VirtualBox 7.0.15 (of the host OS).

    Description

      How to reproduce:

      1. Download VirtualBox 4.3.12 here: https://download.virtualbox.org/virtualbox/4.3.12/VirtualBox-4.3.12-93733-Win.exe (I personally tested this version, but all previous versions starting from 4.1.0 are affected too).
      2. Install it from downloaded exe by double-clicking.
      3. Run VirtualBox from the link on Desktop or Start Menu link.
      4. Create a VM with the random settings (any name, OS type & version, with hard disk of any size/type or even without it at all, etc.) by clicking "New" button on the top toolbar and following the creation wizard step by step. The settings you will chose don't matter, because it's no need to run VM later.
      5. After VM is created, look to the right part which usually contains VM settings and description. Also, open settings by clicking "Settings" button on the top toolbar.

      Expected result:

      VM settings should be properly loaded from XML file, which contains VM configuration. Hence, they should be displayed correctly in the right part. Also, after opening the settings, they should be visible and configurable (able to change).

      Observed result:

      VM settings are actually not loaded correctly from the XML configuration file, so the information is not disapayed in the right part of VirtualBox Manager window ("Information inaccessible" note is specified everywhere instead), and when opening VM settings page, the settings are also not loaded properly (so there are a lot of warnings at the bottom because of improper defaults), and even when trying to set the correct ones and then save them, they are not saved correctly anyway:

      In vbox-4.3.12-ole32.log, the following debug spam appears when trying to load/save VM settings:

      err:(dll/win32/ole32/rpc.c:837) called from wrong apartment, should have been 0x3480000034c
      err:(dll/win32/ole32/rpc.c:837) called from wrong apartment, should have been 0x3480000034c
      err:(dll/win32/ole32/rpc.c:837) called from wrong apartment, should have been 0x3480000034c
      fixme:(dll/win32/rpcrt4/ndr_marshall.c:4665) (0226F8FC,001C093A,0226FA24): stub
      err:(dll/win32/ole32/rpc.c:837) called from wrong apartment, should have been 0x3480000034c
      err:(dll/win32/ole32/rpc.c:837) called from wrong apartment, should have been 0x3480000034c
      err:(dll/win32/ole32/rpc.c:837) called from wrong apartment, should have been 0x3480000034c
      err:(dll/win32/ole32/rpc.c:837) called from wrong apartment, should have been 0x3480000034c
      fixme:(dll/win32/rpcrt4/ndr_marshall.c:4665) (0226F8FC,001C093A,0226FA24): stub
      err:(dll/win32/ole32/rpc.c:837) called from wrong apartment, should have been 0x3480000034c
      err:(dll/win32/ole32/rpc.c:837) called from wrong apartment, should have been 0x3480000034c 

      This occurs because we don't dynamically initialize apartment's oxid member properly when constructing an apartment:

          if (apt->multi_threaded)
          {
              /* FIXME: should be randomly generated by in an RPC call to rpcss */
              apt->oxid = ((OXID)GetCurrentProcessId() << 32) | 0xcafe;
          }
          else
          {
              /* FIXME: should be randomly generated by in an RPC call to rpcss */
              apt->oxid = ((OXID)GetCurrentProcessId() << 32) | GetCurrentThreadId();
          }

      https://git.reactos.org/?p=reactos.git;a=blob;f=dll/win32/ole32/compobj.c;hb=06343fa9a5ba6c341f4b16615cafcbe0003ed128#l652,
      https://git.reactos.org/?p=reactos.git;a=blob;f=dll/win32/ole32/compobj.c;hb=06343fa9a5ba6c341f4b16615cafcbe0003ed128#l657
      (unimplemented code path)

      As a result, the following check in the apartment validation code fails because of apt->oxid and This->oxid mismatching (since they are initialized differently):

      if (This->oxid != oxid)
          return S_FALSE; 

      https://git.reactos.org/?p=reactos.git;a=blob;f=dll/win32/ole32/rpc.c;h=895aa4010cdc36d29fff5582d29c3fc0f0f778e3;hb=06343fa9a5ba6c341f4b16615cafcbe0003ed128#l813

      Since the proper apartment oxid initialization is not impelmented yet even in current Wine: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/combase/apartment.c?ref_type=heads#L387, https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/combase/apartment.c?ref_type=heads#L392, and I doubt I can implement it (and an appropriate rpcss part), I suggest to simply disable that check until the corresponding code is implemented.
      As my tests prove, it fixes the problem described above and does not affect anything other (doesn't make any other regressions), as far as I see (even if there could be some other cases of using this code in ole32). Although it probably might need to run ole32_winetest in both cases (before and after my changes) and then compare test results, to check isn't it regressed anywhere. But nevertheless I'm sure it's worth to commit this and will send a new PR soon.

      Attachments

        Issue Links

          Activity

            People

              Oleg Dubinskij Oleg Dubinskiy
              Oleg Dubinskij Oleg Dubinskiy
              Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated: