Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Commit

Permalink
Enable Linux and BSD compatibility (#49)
Browse files Browse the repository at this point in the history
Summary:
Hey guys, first of all, great work on the framework.

I need to use the framework on FreeBSD servers and it was quite a boomer when I saw it was depending on epoll, so I took some time and came up with this solution using `selectors` instead of `select`, that way things got OS agnostic and selectors should in theory use the most efficient method mechanism available.

I took the care of also adjusting the `poll_mock()` in the tests. Please double check the work there as I was slightly unsure if that was the right way to go.

Luiz

Pull Request resolved: #49

Test Plan:
Test logs for Linux and FreeBSD:
#49 (comment)

Reviewed By: pallotron

Differential Revision: D29100368

Pulled By: skozlov404

fbshipit-source-id: d5b25392a8891deda6903e65989eef0525be759b
  • Loading branch information
lamaral authored and facebook-github-bot committed Jun 16, 2021
1 parent 8d21c08 commit 65fd535
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 20 deletions.
24 changes: 22 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ name: build
on: [push, pull_request]

jobs:
build:

build-linux:
runs-on: ubuntu-latest
strategy:
matrix:
Expand All @@ -26,3 +25,24 @@ jobs:
- name: Test
run: |
make test
build-bsd:
runs-on: macos-latest
strategy:
matrix:
python-version: [ '3.5', '3.6', '3.7', '3.8', '3.9' ]

steps:
- uses: actions/checkout@v2
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies and prepare for test
run: |
python -m pip install --upgrade pip
python -m pip install setuptools --upgrade
python -m pip install flake8
- name: Test
run: |
make test
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ and you are good to go:
# Requirements

* Linux (or any system that supports [`epoll`](http://linux.die.net/man/4/epoll))
* Python 3.x
* BSD (or any system that supports [`kqueue`](https://www.freebsd.org/cgi/man.cgi?query=kqueue&sektion=2))
* Python 3.4+

# Installation

Expand Down
16 changes: 8 additions & 8 deletions fbtftp/base_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import collections
import ipaddress
import logging
import select
import selectors
import socket
import struct
import threading
Expand Down Expand Up @@ -205,8 +205,8 @@ def __init__(
self._listener = socket.socket(self._family, socket.SOCK_DGRAM)
self._listener.setblocking(0) # non-blocking
self._listener.bind((address, port))
self._epoll = select.epoll()
self._epoll.register(self._listener.fileno(), select.EPOLLIN)
self._selector = selectors.DefaultSelector()
self._selector.register(self._listener, selectors.EVENT_READ)
self._should_stop = False
self._server_stats = ServerStats(address, stats_interval_seconds)
self._metrics_timer = None
Expand All @@ -226,7 +226,7 @@ def run(self, run_once=False):
self.run_once()
if run_once:
break
self._epoll.close()
self._selector.close()
self._listener.close()
if self._metrics_timer is not None:
self._metrics_timer.cancel()
Expand Down Expand Up @@ -271,11 +271,11 @@ def run_once(self):
facility to know when data is ready to be retrived from the listening
socket. See http://linux.die.net/man/4/epoll .
"""
events = self._epoll.poll()
for fileno, eventmask in events:
if not eventmask & select.EPOLLIN:
events = self._selector.select()
for key, mask in events:
if not mask & selectors.EVENT_READ:
continue
if fileno == self._listener.fileno():
if key.fd == self._listener.fileno():
self.on_new_data()
continue

Expand Down
20 changes: 11 additions & 9 deletions tests/base_server_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from fbtftp.base_server import BaseServer

MOCK_SOCKET_FILENO = 100
SELECT_EPOLLIN = 1
SELECTORS_EVENT_READ = 1


class MockSocketListener:
Expand Down Expand Up @@ -72,14 +72,16 @@ def setUp(self):
self.interval = 1
self.network_queue = []

def poll_mock(self):
def select_mock(self):
"""
mock the select.epoll.poll() method, returns an iterable containing a
list of (fileno, eventmask), the fileno constant matches the
MockSocketListener.fileno() method, eventmask matches select.EPOLLIN
"""
if len(self.network_queue) > 0:
return [(MOCK_SOCKET_FILENO, SELECT_EPOLLIN)]
obj = lambda: None
obj.fd = MOCK_SOCKET_FILENO
return [(obj, SELECTORS_EVENT_READ)]
return []

def prepare_and_run(self, network_queue):
Expand All @@ -104,10 +106,10 @@ def prepare_and_run(self, network_queue):
server._server_stats.increment_counter.assert_called_with("process_count")
return server._handler

@patch("select.epoll")
def testRRQ(self, epoll_mock):
@patch("selectors.DefaultSelector")
def testRRQ(self, selector_mock):
# link the self.poll_mock() method with the select.epoll patched object
epoll_mock.return_value.poll.side_effect = self.poll_mock
selector_mock.return_value.select.side_effect = self.select_mock
self.network_queue = [
# RRQ + file name + mode + optname + optvalue
b"\x00\x01some_file\x00binascii\x00opt1_key\x00opt1_val\x00"
Expand Down Expand Up @@ -171,10 +173,10 @@ def testCallbackException(self):
stats_callback.side_effect = Exception("boom!")
self.start_timer_and_wait_for_callback(stats_callback)

@patch("select.epoll")
def testUnexpectedOpsCode(self, epoll_mock):
@patch("selectors.DefaultSelector")
def testUnexpectedOpsCode(self, selector_mock):
# link the self.poll_mock() emthod with the select.epoll patched object
epoll_mock.return_value.poll.side_effect = self.poll_mock
selector_mock.return_value.select.side_effect = self.select_mock
self.network_queue = [
# RRQ + file name + mode + optname + optvalue
b"\x00\xffsome_file\x00binascii\x00opt1_key\x00opt1_val\x00"
Expand Down

0 comments on commit 65fd535

Please sign in to comment.