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

Lua uvFifoListener ffi #1156

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

caldog20
Copy link
Contributor

Adds ability to spin up a uvFifoListener (similar to using uvFifo in Lua) but as a server instead of a client.

  • Garbage Collection for the fifo inside the wrapper needs adjusting
  • UvFifoListener needs some pending changes to move m_async and tidy up a few things inside the class. This can be done after the current Lua changes are finalized

@codecov
Copy link

codecov bot commented Dec 14, 2022

Codecov Report

Base: 16.37% // Head: 16.38% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (6d1eaa6) compared to base (d43e9e9).
Patch coverage: 10.00% of modified lines in pull request are covered.

❗ Current head 6d1eaa6 differs from pull request most recent head 19dbddd. Consider uploading reports for the commit 19dbddd to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1156   +/-   ##
=======================================
  Coverage   16.37%   16.38%           
=======================================
  Files         362      363    +1     
  Lines      115794   115871   +77     
=======================================
+ Hits        18962    18980   +18     
- Misses      96832    96891   +59     
Impacted Files Coverage Δ
src/lua/luafile.h 0.00% <0.00%> (ø)
src/support/uvfile.cc 8.06% <0.00%> (-0.06%) ⬇️
src/support/uvfile.h 16.36% <0.00%> (-0.62%) ⬇️
src/lua/luafile.cc 32.96% <12.12%> (-4.62%) ⬇️
src/core/r3000a.h 50.66% <0.00%> (-4.41%) ⬇️
src/core/psxmem.cc 64.73% <0.00%> (-3.67%) ⬇️
src/cdrom/iec-60908b.h 18.18% <0.00%> (ø)
src/cdrom/cdriso-ecm.cc 0.00% <0.00%> (ø)
src/cdrom/iec-60908b.cc 0.00% <0.00%> (ø)
third_party/iec-60908b/edcecc.c 0.00% <0.00%> (ø)
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.


_callback = function(fifo) end

if cb ~= nil then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we check for cb == nil before, this check isn't really useful.

Also, we're going to overwrite _callback afterward. And it should be declared as local.

@@ -73,6 +74,22 @@ LuaFile* subFile(LuaFile* wrapper, uint64_t start, int64_t size) {
}
LuaFile* uvFifo(const char* address, int port) { return new LuaFile(new PCSX::UvFifo(address, port)); }

LuaServer* uvFifoListener() {return new LuaServer(new PCSX::UvFifoListener());}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need a function to delete a LuaServer. It's not simple as it needs to schedule a stop if the server is running, and then delete in the async close callback.

}

void stopListener(LuaServer* server) {
server->m_listener->stop();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably need a status variable to avoid double stops. It needs to be "STARTED", "STOPPING", "STOPPED".

}
-- Use a proxy instead of doing this on the wrapper directly using ffi.gc, because of a bug in LuaJIT,
-- where circular references on finalizers using ffi.gc won't actually collect anything.
debug.setmetatable(listener._proxy, { __gc = function() C.stopListener(listener._wrapper) end })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should (1) call the deleter of the class instead, and (2) it should free the callback in Lua.

@caldog20 caldog20 marked this pull request as ready for review January 1, 2023 16:55
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.

2 participants