mdsplus icon indicating copy to clipboard operation
mdsplus copied to clipboard

perror is not effective in win32

Open LiyeeW opened this issue 3 years ago • 14 comments

In \mdsplus-alpha\mdstcpip\mdsipshr\ParseCommand.c:284, mdsplus trys to use perror get last error. However, perror() used in win32 can only get the string "no error", which should be GetLastError().

LiyeeW avatar Jan 11 '22 01:01 LiyeeW

Furthermore, what confuses me is that official website doesn't recommend to duplicate socket by calling DuplicateHandle(), where I always get an unknown error, which can't be shown by perror(). The words in https://docs.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-duplicatehandle are as follows:

Sockets. No error is returned, but the duplicate handle may not be recognized by Winsock at the target process. Also, using DuplicateHandle interferes with internal reference counting on the underlying object. To duplicate a socket handle, use the WSADuplicateSocket function.

Could you please check this calling? I have no idea how to get my mdsplus server connected, sincerely hoping you can give some help or ideas.

LiyeeW avatar Jan 11 '22 02:01 LiyeeW

In \mdsplus-alpha\mdstcpip\mdsipshr\ParseCommand.c:284,

are you sure about the file:line? ParseCommand.c does not seem to have 284 lines. Given the second comment, WSA* command wont set errno so perror has no effect. instead we have a function

https://github.com/MDSplus/mdsplus/blob/3d509f98137f272cbeac9893ccd963a128d0477f/_include/socket_port.h#L142

which wraps around WSALastError() under windows.

The DuplicateHandel is it not used for pipes? Maybe you can reference to code here on github to clarify where we may improve the code or help you out otherwise.

zack-vii avatar Jan 11 '22 07:01 zack-vii

Hi Liyee, I agree with Timo, could it be that you are referring to a different code? There is not reference to perror() or get last error. Could you point us to the code that you are thinking needs improvement? Cheers.

santorofer avatar Jan 11 '22 17:01 santorofer

Also, yes, DuplicatedHandle() is being used in the context of C pipes. Not with sockets.

santorofer avatar Jan 11 '22 20:01 santorofer

Thank you for your reply and patience. I indeed made a mistake. Where I get trouble is at , so sorry.

\mdsplus-alpha\mdstcpip\io_routines\ioroutinesx.h:284

Code is like this, which I downloaded from github one or two weeks ago, and we can find it in the newest code.

if(!DuplicateHandle(OpenProcess(PROCESS_ALL_ACCESS, TRUE, ppid), (HANDLE)psock, GetCurrentProcess(), (HANDLE *)&h, PROCESS_ALL_ACCESS, TRUE, DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) { fprintf(stderr, "Attempting to duplicate socket from pid %d socket %d\n", ppid, (int)psock); perror("Error duplicating socket from parent"); exit(EXIT_FAILURE); }

I think the function is being used in the Context of socket, because of (HANDLE)psock here. On offifcial website's page of DuplicateHandle, it also says:

If the function fails, the return value is zero. To get extended error information, call GetLastError. ... ... You should not use DuplicateHandle to duplicate handles to the following objects: Sockets. No error is returned, but the duplicate handle may not be recognized by Winsock at the target process.

In my mdsplus' log, it shows: "Error duplicating socket from parent: No error"

Given my connection breaks down here, the return value of DuplicateHandle ought to be zero, and ought to have error information. However, I'm not sure whether my error comes from the second sentence on official website, because if it's caused by it, there should be "no error returned".

So using GetLastError is the main solution to find out the reason, I think.

LiyeeW avatar Jan 12 '22 02:01 LiyeeW

Hi Liyee, Thank you for the correction, I think you are into something. In socket_port.h, WSA* seems to be used extensibly, though. It looks to me that perror() actually shows "No error", exactly what the DuplicateHandle() documentation refers too. I will try to reproduce the issue. In your case, how did you encounter the problem?

santorofer avatar Jan 12 '22 05:01 santorofer

i agree, good find. we should use the wsa / socket_port.h error report here s well. I have the feeling, this wont necesarely fix your issue but the error code will hopefully point you in the right direction. we could prepare a fix today. maybe you can try it out before we merge it.

Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: Fernando Santoro @.> Sent: Wednesday, January 12, 2022 6:07:04 AM To: MDSplus/mdsplus @.> Cc: Timo Schroeder @.>; Comment @.> Subject: Re: [MDSplus/mdsplus] perror is not effective in win32 (Issue #2439)

Hi Liyee, Thank you for the correction, I think you are into something. In socket_port.h, WSA* seems to be used extensibly, though. It looks to me that perror() actually shows "No error", exactly what the DuplicateHandle() documentation refers too. I will try to reproduce the issue. In your case, how did you encounter the problem?

— Reply to this email directly, view it on GitHubhttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMDSplus%2Fmdsplus%2Fissues%2F2439%23issuecomment-1010639860&data=04%7C01%7C%7Cbb27d13843834a5320b608d9d5896085%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637775608262593675%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=hMcbW4GriKaGWynRuJG%2F9oj64MTFP9ZrM7dgfzwai0U%3D&reserved=0, or unsubscribehttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABPRFLRH77G7TYRL2BEM3T3UVUD7RANCNFSM5LVDLVDQ&data=04%7C01%7C%7Cbb27d13843834a5320b608d9d5896085%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637775608262593675%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=hcx7Ax7N9WD%2Fgz1IURYJl70a7Kn4a4YpiQ%2BFADPjh4o%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7C%7Cbb27d13843834a5320b608d9d5896085%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637775608262593675%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=8WLJu1RCN%2FsM4YyOSJfHXur3irRwjxMseSuAISTM7oc%3D&reserved=0 or Androidhttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7C%7Cbb27d13843834a5320b608d9d5896085%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637775608262593675%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ATfGWjLTIiskotR%2Fk0NetoEily0gSA3dJm%2Fs%2BqGw8Kk%3D&reserved=0. You are receiving this because you commented.Message ID: @.***>

zack-vii avatar Jan 12 '22 07:01 zack-vii

Hi Liyee, We will be working on fixing this issue. Now, in order to do that, could you give us a detail step-by-step procedure to reproduce error that you are seeing? That will help us to test the changes in a faster way. As soon as we can do that, we can promote the changes to alpha and you can get it from there to try it out.

santorofer avatar Jan 12 '22 17:01 santorofer

hmm there are to issues to resolve:

  • the code in question is shared by udt and tcp (for udt one could #if SOCKET == UDT_SOCKET)
  • WSADuplicateSocketA does not return a socket but an info structure that one can use to create one, how do we test this.

It is important to understand how this works before we generate a half baked solution.

so far i did

+#if SOCKET == UDT_SOCKET
   if (!DuplicateHandle(
           OpenProcess(PROCESS_ALL_ACCESS, TRUE, ppid),
           (HANDLE)psock, GetCurrentProcess(), (HANDLE *)&h,
           PROCESS_ALL_ACCESS, TRUE,
           DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS))
   {
     fprintf(stderr, "Attempting to duplicate socket from pid %d socket %d\n",
             ppid, (int)psock);
     perror("Error duplicating socket from parent");
     exit(EXIT_FAILURE);
   }
+#else
+  WSAPROTOCOL_INFOA protocolInfo;
+  if (!WSADuplicateSocketA(
+          OpenProcess(PROCESS_ALL_ACCESS, TRUE, ppid),
+          psock, GetCurrentProcess(), &protocolInfo))
+  {
+    fprintf(stderr, "Attempting to duplicate socket from pid %d socket %d\n",
+            ppid, (int)psock);
+    print_socket_error("Error duplicating socket from parent");
+    exit(EXIT_FAILURE);
+  }
+  SOCKET h = WSASocketA(AF_T, SOCK_STREAM, &protocolInfo, 0, 0);
+#endif

zack-vii avatar Jan 12 '22 18:01 zack-vii

Thank for all your work very much. But I tried to reproduce this error on another win10 PC but failed, which broke down at c->io->recv, another error that I haven't known clear. I remembered this issue's error is like this: I downloaded stable package from website, and installed it without changing path. Then I clicked "install....8000", and checked 8000 had been listening. The I use Python: from MDSplus import connection c = connection.Connection("127.0.0.1") Then the error happened, and I could find newly log in C:, the context of which was like

Error duplicating socket from parent: No error Attempting to duplicate ... ...

The above from using python to finding log can reproduce stably on the first win10 PC. I think this is all I know, though I think it seems to have a little value... ...

LiyeeW avatar Jan 12 '22 23:01 LiyeeW

Liyee, thank you for your comments and steps to reproduce it. Timo, do you think is necessary to make a distinction between UDT or TPC sockets? Because it seems that DuplicateHandle() is only used to duplicate kernel object's handles among processes. Maybe we can still use WSADuplicateSocketA() for both UDT and TCP? I can try testing both cases though.

santorofer avatar Jan 12 '22 23:01 santorofer

Hi Lyee, I'm having problems reproducing your issue. Mainly:

  • did you setup an mdsip server on Windows? if so, how did you do that?
  • what do you mean by "I clicked 'install .... 8000'"?
  • Could you let me know all the steps that you follow, in full detailed? Cheers!

santorofer avatar Jan 13 '22 18:01 santorofer

Hi santorofer, I did setup an music server on Windows by clicking the shortcut called maybe "install mdsplus data sever on 8000 port", which may be a command of "mdsip_service.exe", and that is created by setup package automatically. And after clicking that shortcut, I check in cmd by "netstate ", and I'm sure the server is successfully listening on port 8000.

I'm sorry that I may have no more details.After making sure the server is listening, I use python, fail to connect and find out newly log. By the way, I can see new socket, which is WAIT or ESTABLISHED maybe, of 8000 port by "netstat" at the moment I fail to connect, and the new socket will soon be CLOSED maybe.

For some reason I couldn't use that computer until next month, so I use "maybe" to tell the name I'm not sure, hoping it can be useful.

LiyeeW avatar Jan 14 '22 03:01 LiyeeW

Hello Liyee, We are working on understanding why the Connection method to localhost seems to fail in Windows. But I wanted to let you know that you could also use the following to access a local tree.

Please, take a look at the following wiki page to have more information:

https://www.mdsplus.org/index.php/Documentation:Tutorial:MdsObjects https://www.mdsplus.org/index.php?title=Documentation:Tutorial:MdsObjects&open=1624466399993193955313&page=Tutorials%2FAPIs%2Fpython#Some_working_examples

You should, firstly, be sure that the environment variable of your tree is well set, so that it points to the location of the tree model (eg. the MDSplus installation has the following example tree: main_path=C:\Program Files\MDSplus\trees).

------------------------------------------------------------------------
Option A (functional access to the data):
------------------------------------------------------------------------
>>> import MDSplus
>>> MDSplus.Data.execute('treeopen("main", -1)')
265389633
>>> MDSplus.Data.execute('SIGNAL')
Build_Signal([0.,1.,2.,3.,4.,5.,6.,7.,8.,9.,10.,11.,12.,13.,14.,15.,16.,17.,18.,


------------------------------------------------------------------------
Option B (object API interface):
------------------------------------------------------------------------
>>> import MDSplus
>>> tree=MDSplus.Tree('main', -1)
>>> tree
Tree("MAIN",-1,"Normal")
>>> tree.dir()
CHILD           STRUCTURE
SUBTREE         SUBTREE
MEMBER          NUMERIC
NUMERIC         NUMERIC
SIGNAL          SIGNAL
TEXT            TEXT
>>> s=tree.SIGNAL.data()
>>> s
array([  0.,   1.,   2.,   3.,   4.,   5.,   6.,   7.,   8.,   9.,  10.,
        11.,  12.,  13.,  14.,  15.,  16.,  17.,  18.,  19.,  20.,  21.,
        22.,  23.,  24.,  25.,  26.,  27.,  28.,  29.,  30.,  31.,  32.,
        33.,  34.,  35.,  36.,  37.,  38.,  39.,  40.,  41.,  42.,  43.,
        44.,...

As I was saying, we are working to have the connection.Connection() working. We will let you know.

santorofer avatar Jan 24 '22 18:01 santorofer