Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 105 #106

Merged
merged 14 commits into from
Oct 28, 2023
Merged

Issue 105 #106

merged 14 commits into from
Oct 28, 2023

Conversation

mutouyun
Copy link
Owner

No description provided.

@jayprakashkumar1
Copy link

@mutouyun I think you shouldn't directly make the prefix Global\ in the ipc name because by changing it to Global\ the standard user can't create the ipc if they need to. Creating ipc will raise ERROR_ACCESS_DENIED(error code 5). Only the administrator/System account can create then only other users can access it.

So if someone just wants ipc in two user-mode processes running as a standard user then if the Globall\ prefix is used then they can't do it because creating with Global needs admin/system permission.

If someone wants ipc between service and GUI then they can prefix with Global\ otherwise just keep it as it is without Globall\

So make it somehow reliable for all the use cases.

Suggestion: You can provide this feature(Globall\) by user-provided option while creating ipc.
If the user needs Global then only prefix with Global\ otherwise keep it as it is.

Thanks :)

@mutouyun
Copy link
Owner Author

Sounds like things are getting complicated... This will affect the user interface, I first try to write a sample demo to test the behavior.

@mutouyun
Copy link
Owner Author

I submit a new change set, prefixed with Globall\ depending on the user group. To be honest, I don't like this, because the prefix is implicit, and users may not get any errors if they can't communicate. I should still take your advice. Before I do that, let me think carefully about how to adjust the interface.

@jayprakashkumar1
Copy link

Let's say the system account inside the service creates the ipc for writing and the standard user in other sessions (GUI desktop app) opens the ipc then the ipc name will be different since you are adding a prefix Global\ only for the admin/system account. So in this case no communication happens.

Also if the admin user (name will be prefixed with Globall\) & standard user use the ipc then still be no communication because names are different for both.

So I think adding the prefix Global\ for the Admin/System account won't work.
Therefore You can just pass a prefix while creating the ipc this way user will have the option to use between service & GUI by just passing the prefix Global\

For other cases no need to pass the Global\ prefix.
Simple I think.

Please check it and make changes.
Thanks :)

@mutouyun
Copy link
Owner Author

Yeah, you're right. This commit 1753178 is just to test the behavior of different permissions.

The difficulty now is that in the delivery of large message blocks, they are allocated through the same central pool, and the context does not follow the channel object. I thought about a possible solution, and I will change the code and test it tomorrow.

@mutouyun mutouyun marked this pull request as draft September 24, 2023 08:20
@mutouyun
Copy link
Owner Author

I commited a draft version, the code changes too much and there are not enough unit tests on the critical path. You could test whether the behavior is as expected first.

I don't have much time to do open source library development during a normal workday, in addition, next Friday (September 29) will start the Mid-Autumn Festival and National Day holiday in China. I will go back to my hometown and spend time with my family, and I may have no time to touch the computer... So if you have time, you could help me add some unit tests in this pr, this will help me a lot.

@jayprakashkumar1
Copy link

OK definitely will check it and add unit tests as much as possible. By the way, sure enjoy your weekend/holidays with your families.
Thanks :)

@jayprakashkumar1
Copy link

Hi @mutouyun , First of all so sorry about the long time. I was involved in some sudden things which was out of control for me.
Now I am back and will see now.

I have checked it works between the client(as a standard user) and the service(Local system account). But the problem is when I start the client first and then start service then THERE IS NO COMMUNICATION. Failed on both ends.

Even I tried as reconnect() method but still no communication. Simply failed.
It should work as soon as the service starts if the client is already trying to send it.

So how can we deal with this situation?

I have modified some code to check it:

Service part code:

using msg_que_t = ipc::chan<ipc::relat::multi, ipc::relat::multi, ipc::trans::broadcast>;
msg_que_t ipc_r{ ipc::prefix{"Global\\"}, "service ipc r" };
msg_que_t ipc_w{ ipc::prefix{"Global\\"}, "service ipc w" };

DWORD WINAPI ServiceWorkerThread (LPVOID lpParam) {
    OutputDebugString(_T("My Sample Service: ServiceWorkerThread: Entry"));
   /* ipc::channel ipc_r{ipc::prefix{"Global\\"}, "service ipc r", ipc::sender};
    ipc::channel ipc_w{ipc::prefix{"Global\\"}, "service ipc w", ipc::receiver};*/
    
    if (!ipc_r.reconnect(ipc::sender)) {
        OutputDebugString(_T("sender connect failed"));
    }
    if (!ipc_w.reconnect(ipc::receiver)) {
        OutputDebugString(_T("receiver connect failed"));
    }

    //  Periodically check if the service has been requested to stop
    while (WaitForSingleObject(g_ServiceStopEvent, 0) != WAIT_OBJECT_0) {        
        /* 
         * Perform main service function here
         */
        if (!ipc_r.send("Hello, World!")) {
            OutputDebugString(_T("My Sample Service: send failed."));
            if (!ipc_r.reconnect(ipc::sender)) {
                OutputDebugString(_T("sender reconnect failed"));
            }
            else {
                OutputDebugString(_T("Reconnection as sender established!"));
            }
        }
        else {
            OutputDebugString(_T("My Sample Service: send [Hello, World!]"));
            auto msg = ipc_w.recv(1000);
            if (msg.empty()) {
                OutputDebugString(_T("My Sample Service: recv error"));
                if (!ipc_w.reconnect(ipc::receiver)) {
                    OutputDebugString(_T("receiver reconnect failed"));
                }
                else {
                    OutputDebugString(_T("Reconnection as receiver established!"));
                }
            } else {
                OutputDebugStringA((std::string{"My Sample Service: recv ["} + msg.get<char const *>() + "]").c_str());
            }
        }
        Sleep(3000);
    }

    OutputDebugString(_T("My Sample Service: ServiceWorkerThread: Exit"));

    return ERROR_SUCCESS;
}

client part code:

using msg_que_t = ipc::chan<ipc::relat::multi, ipc::relat::multi, ipc::trans::broadcast>;
msg_que_t ipc_r{ ipc::prefix{"Global\\"}, "service ipc r" };
msg_que_t ipc_w{ ipc::prefix{"Global\\"}, "service ipc w" };

int _tmain (int argc, TCHAR *argv[]) {
    _tprintf(_T("My Sample Client: Entry\n"));
    try {
       /* ipc::channel ipc_r{ ipc::prefix{"Global\\"}, "service ipc r", ipc::receiver };
        ipc::channel ipc_w{ ipc::prefix{"Global\\"}, "service ipc w", ipc::sender };*/
        while (1) {
            auto msg = ipc_r.recv();
            if (msg.empty()) {
                _tprintf(_T("My Sample Client: message recv error\n"));
                if (!ipc_r.reconnect(ipc::receiver)) {
                    _tprintf(_T("receiver reconnect failed\n"));
                }
                else {
                    _tprintf(_T("Reconnection as receiver established!\n"));
                }
                Sleep(1000);
                continue;
                //return -1;
            }
            printf("My Sample Client: message recv: [%s]\n", (char const*)msg.data());
            while (!ipc_w.send("Copy.")) {
                _tprintf(_T("My Sample Client: message send error\n"));
                if(!ipc_w.reconnect(ipc::sender)) {
                    _tprintf(_T("sender reconnect failed\n"));
                }
                else {
                    _tprintf(_T("Reconnection as sender established!\n"));
                }
                Sleep(1000);
            }
            _tprintf(_T("My Sample Client: message send [Copy]\n"));
            Sleep(1000);
           
        }
        _tprintf(_T("My Sample Client: Exit\n"));
    }
    catch (std::exception& e) {
        printf("Exception: [%s]\n", e.what());
    }
    return 0;
}

@mutouyun
Copy link
Owner Author

Is this the error that occurs when you execute client first?

My Sample Client: Entry
fail CreateFileMapping/OpenFileMapping[5]: Global\__IPC_SHM__CC_CONN__service ipc r_WAITER_COND__COND_SHM_
fail get_mem: invalid id (null)
fail CreateFileMapping/OpenFileMapping[5]: Global\__IPC_SHM__WT_CONN__service ipc r_WAITER_COND__COND_SHM_
fail get_mem: invalid id (null)
fail CreateFileMapping/OpenFileMapping[5]: Global\__IPC_SHM__RD_CONN__service ipc r_WAITER_COND__COND_SHM_
fail get_mem: invalid id (null)
fail CreateFileMapping/OpenFileMapping[5]: Global\__IPC_SHM__AC_CONN__service ipc r
fail get_mem: invalid id (null)
fail CreateFileMapping/OpenFileMapping[5]: Global\__IPC_SHM__CA_CONN__
fail get_mem: invalid id (null)
[cc_acc] acquire failed: Global\__IPC_SHM__CA_CONN__
fail CreateFileMapping/OpenFileMapping[5]: Global\__IPC_SHM__QU_CONN__64__8__service ipc r
fail get_mem: invalid id (null)
fail CreateFileMapping/OpenFileMapping[5]: Global\__IPC_SHM__CC_CONN__service ipc w_WAITER_COND__COND_SHM_
fail get_mem: invalid id (null)
fail CreateFileMapping/OpenFileMapping[5]: Global\__IPC_SHM__WT_CONN__service ipc w_WAITER_COND__COND_SHM_
fail get_mem: invalid id (null)
fail CreateFileMapping/OpenFileMapping[5]: Global\__IPC_SHM__RD_CONN__service ipc w_WAITER_COND__COND_SHM_
fail get_mem: invalid id (null)
fail CreateFileMapping/OpenFileMapping[5]: Global\__IPC_SHM__AC_CONN__service ipc w
fail get_mem: invalid id (null)
fail CreateFileMapping/OpenFileMapping[5]: Global\__IPC_SHM__CA_CONN__
fail get_mem: invalid id (null)
[cc_acc] acquire failed: Global\__IPC_SHM__CA_CONN__
fail CreateFileMapping/OpenFileMapping[5]: Global\__IPC_SHM__QU_CONN__64__8__service ipc w
fail get_mem: invalid id (null)
My Sample Client: message recv error
fail WaitForSingleObject[6]: 0xFFFFFFFF
fail ReleaseMutex[6]
fail WaitForSingleObject[6]: 0xFFFFFFFF
fail ReleaseMutex[6]
fail WaitForSingleObject[6]: 0xFFFFFFFF
fail ReleaseMutex[6]
fail WaitForSingleObject[6]: 0xFFFFFFFF
fail ReleaseMutex[6]
fail WaitForSingleObject[6]: 0xFFFFFFFF
fail ReleaseMutex[6]
fail WaitForSingleObject[6]: 0xFFFFFFFF
fail ReleaseMutex[6]

Run the server first, then the client, and the result is fine, but not the other way around, right?
The reason it went wrong is because of this: https://stackoverflow.com/questions/3999157/system-error-0x5-createfilemapping
The reconnect function does not take care of the case where the shared memory creation fails, which should be fixed.
I updated the code and adjusted the demo code of the client, you can see if it meets the expectations.

@jayprakashkumar1
Copy link

Thanks @mutouyun for fixing the code. yes, the same error was occurring when I executed the client first as posted by you.

I checked Now and it works as expected. Will further check by tonight and update tomorrow then you can merge to master.

And again Thanks a lot :)

@mutouyun mutouyun marked this pull request as ready for review October 23, 2023 12:16
@jayprakashkumar1
Copy link

Hi @mutouyun ! One thing I have noticed while testing is that if the receiver service is stopped then the client sender fails with an error: fail: send, there is no receiver on this connection. which is good. but If I run the same code as the client-server without window service then if the receiver stops then still client side continues sending msg successfully which shouldn't.
The rest working fine.

code:

int _tmain(int argc, TCHAR* argv[]) {
    
    if (argc != 2) {
        printf("usage: win_client.exe r/s");
        return 1;
    }
    std::string command = argv[1];
    //sender part
    if (command == "s") {
        ipc::channel ipc_w{ ipc::prefix{"Global\\"}, "service ipc Name", ipc::sender };
        ipc_w.reconnect(ipc::sender);

        while (1) {
            static UINT count = 0;
            static std::string msg = "";
            msg = "msg [" + std::to_string(++count) + "] from client side";

            if (ipc_w.send(msg)) {
                printf("My Sample Client: message send success: [%s]\n", msg.c_str());
            }
            else {
                _tprintf(_T("My Sample Client: message send Failed\n"));
                if (!ipc_w.reconnect(ipc::sender)) {
                    _tprintf(_T("My Sample Client: Reconnection to server failed!\n"));
                }
                else {
                    _tprintf(_T("My Sample Client: Reconnection to server established!\n"));
                    if (ipc_w.send(msg)) {
                        _tprintf(_T("My Sample Client: message send success\n"));
                    }
                    else {
                        _tprintf(_T("My Sample Client: message send Failed\n"));
                        ipc_w.disconnect();
                    }
                }
            }
            Sleep(1000);
        }
    }
    //reciever part
    else if (command == "r") {
        ipc::channel ipc_r{ ipc::prefix{"Global\\"}, "service ipc Name", ipc::receiver };
        ipc_r.reconnect(ipc::receiver);

        while (1) {
            auto msg = ipc_r.recv(1000);
            if (msg.empty()) {
                printf("My Sample server: recv error\n");
            }
            else {
                printf("My Sample server: message recived success: [%s]\n", msg.get<char const*>());
            }
           
            Sleep(1000);
        }
        _tprintf(_T("My Sample server: Exit\n"));
    }
    return 0;
}

@mutouyun
Copy link
Owner Author

This is because the service exits safely via an event signal, and the destructor of the channel is automatically triggered when the process exits. However, when you manually stop the receiver via the command line "CTRL+C", the SIGINT signal does not automatically trigger the destructor, but exits the process directly.

So you should add the corresponding signal capture and handle the exit behavior in it.

#include <signal.h>

ipc::channel *ipc__ = nullptr;

int _tmain(int argc, TCHAR* argv[]) {
    
    if (argc != 2) {
        printf("usage: win_client.exe r/s");
        return 1;
    }
    auto exit = [](int sig) {
      if (ipc__ != nullptr) ipc__->disconnect();
      ::exit(sig);
    };
    ::signal(SIGINT, exit);
    ::signal(SIGBREAK, exit);

    //sender part
    if (_tcsnccmp(argv[1], _T("s"), 1) == 0) {
        ipc::channel ipc_w{ ipc::prefix{"Global\\"}, "service ipc Name", ipc::sender };
        ipc__ = &ipc_w;
        // ...
    }
    //reciever part
    else if (_tcsnccmp(argv[1], _T("r"), 1) == 0) {
        ipc::channel ipc_r{ ipc::prefix{"Global\\"}, "service ipc Name", ipc::receiver };
        ipc__ = &ipc_r;
        // ...
    }
}

@jayprakashkumar1
Copy link

jayprakashkumar1 commented Oct 28, 2023

OK, I got it. But the process can be terminated in other ways as well. Simply end the task from the task manager. Or even let's assume the receiver crashes in some way so in all such cases, the receiver process exits abnormally and it doesn't trigger the destructor, but exits the process directly. so in this scenario still sender will still continue sending successfully.

Is there any way to handle these scenarios as well for better functionality?

@mutouyun
Copy link
Owner Author

Because the current code design is difficult to accurately determine whether the connection is broken, so there is no way to completely avoid this, I hope to solve this problem on the refactoring branch.

@jayprakashkumar1
Copy link

Ok great. You can merge now I think. Thanks

@mutouyun mutouyun merged commit ac54be7 into master Oct 28, 2023
2 checks passed
@mutouyun mutouyun deleted the issue-105 branch October 28, 2023 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No ipc communication between gui(logged in user) and windows services(Local system account)
2 participants