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

Implement running as service as well as standalone exe #33

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

ahmubashshir
Copy link

  • Organize source files
  • Separate syscall wrappers and code
  • Prepare for implementing service
  • Add placeholder service management functions
  • Implement service (un)install
  • Implement Service Runner
  • Add compiled resource support
  • Implement Service Error reporter
  • Strip output by default
  • Update Status messages and formatting

@0e4ef622
Copy link
Owner

Thanks for the PR! Most of this looks ok, although there's an issue I've found. After a game closes but before the service is stopped, the process uses a lot of CPU (~50-70%). I'm pretty sure this is caused by the lack of cleanup in the original code, so the original pipe handle is never closed and the iServerLoop fails to create the pipe, causing an infinite loop, although I'm not sure how to view logs so not certain.

Basically, iServerLoop needs cleanup code.

@ahmubashshir
Copy link
Author

ahmubashshir commented Jan 13, 2023 via email

@Roadhog360
Copy link

Roadhog360 commented Mar 3, 2023

What does running this as a service mean and how can I do that? I've compiled it but it appears to just do the same thing as the regular exe.

@0e4ef622
Copy link
Owner

0e4ef622 commented Mar 3, 2023

What does running this as a service mean and how can I do that? I've compiled it but it appears to just do the same thing as the regular exe.

If you run ./bridge.exe install once, then afterwards it should start/stop automatically. It should also be able to accept another connection once the first one closes, although that part doesn't seem to quite work.

@Roadhog360
Copy link

Roadhog360 commented Mar 4, 2023

./bridge.exe install doesn't work because bridge.exe is not a command and the compiler outputs winediscordipcbridge.exe, not bridge.exe. Trying wine winediscordipcbridge.exe install appears to do nothing and has the same exact result as running the .exe with no arguments.

Even though the .exe closes when the first connection closes, if the part where it's supposed to take another connection is supposedly broken then unfortunately as far as what I am looking for, we're back to square one.

@ahmubashshir
Copy link
Author

ahmubashshir commented Mar 5, 2023

./bridge.exe install doesn't work because bridge.exe is not a command and the compiler outputs winediscordipcbridge.exe, not bridge.exe.

is this part supposed to be sarcasm?

Even though the .exe closes when the first connection closes, if the part where it's supposed to take another connection is supposedly broken then unfortunately as far as what I am looking for, we're back to square one.

wine ./wdipcbridge.exe install for the first time should add the IPC bridge to services of the wineprefix... next time you run something in that WINEPREFIX it'll be able to communicate with discord.

You can test whether it works by using examples/send-presence.exe from this.

Remember, you need to run wine ./wdipcbridge.exe install for each WINEPREFIX you want to have this on.

edit: Did you apply this patch before building it?

@ahmubashshir
Copy link
Author

@0e4ef622 btw, I don't know much about how M$ pipes work, and how can I make it behave like unix sockets (multiple streams). If we can implement multi-stream support in iListenPipe, every client will be able to talk to discord, currently the first one does all the talking afair.

@0e4ef622
Copy link
Owner

0e4ef622 commented Mar 5, 2023

how can I make it behave like unix sockets (multiple streams).

You can set the nMaxInstances parameter in CreateNamedPipe, but also I believe https://github.com/openglfreak/winestreamproxy is a project which already does that.

@Roadhog360
Copy link

Roadhog360 commented Mar 10, 2023

I think I may know why I got confused earlier. The repo for this PR does not include the main.c file, causing the compile instructions to fail so I copied it from the main repository instead, could that cause issues?

Edit: Yes, it absolutely did. Me discussing both here and on my issue caused me to miss a major important step so I'll just discuss this on my issue to reduce the possibility of confusion.

@sersorrel
Copy link

this unfortunately no longer compiles with GCC 14:

In file included from src/main.c:2:
src/main.c: In function 'main':
src/main.c:15:25: error: implicit declaration of function 'printf' [-Wimplicit-function-declaration]
   15 |                 return (_tprintf(
      |                         ^~~~~~~~
src/main.c:6:1: note: include '<stdio.h>' or provide a declaration of 'printf'
    5 | #include "service-manager.h"
  +++ |+#include <stdio.h>
    6 | #define ARGV1(X) (argc > 1 && lstrcmpi( argv[1], TEXT(X)) == 0)

the following patch should fix the error and also several warnings:

diff --git a/src/ipc.c b/src/ipc.c
index 17533d0..dca3b2b 100644
--- a/src/ipc.c
+++ b/src/ipc.c
@@ -43,7 +43,7 @@ int iServerMain(BOOL bStandaloneArg)

 	if (hPipe == INVALID_HANDLE_VALUE)
 	{
-		INFO("CreateNamedPipe failed, GLE=%d.\n", GetLastError());
+		INFO("CreateNamedPipe failed, GLE=%lu.\n", GetLastError());
 		return -1;
 	}

@@ -122,7 +122,7 @@ PRIVATE int iListenPipe()

 	if (hThread == NULL)
 	{
-		INFO("CreateThread failed, GLE=%d.\n", GetLastError());
+		INFO("CreateThread failed, GLE=%lu.\n", GetLastError());
 		return 1;
 	}

@@ -149,7 +149,7 @@ PRIVATE int iListenPipe()
 			return 1;
 		}

-		INFO("%d bytes w->l\n", bytes_read);
+		INFO("%lu bytes w->l\n", bytes_read);
 		/* pass -D__WDBRIDGE_DUMP_PIPE to gcc
 		* to dump the actual data being passed
 		* from the pipe to the socket */
diff --git a/src/main.c b/src/main.c
index b5fe2e8..68dee71 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1,6 +1,7 @@
 #include <windows.h>
 #include <tchar.h>
 #include <shlwapi.h>
+#include <stdio.h>
 #include "server.h"
 #include "service-manager.h"
 #define ARGV1(X) (argc > 1 && lstrcmpi( argv[1], TEXT(X)) == 0)

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.

4 participants