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

Support CCL #74

Merged
merged 1 commit into from
Jun 21, 2019
Merged

Support CCL #74

merged 1 commit into from
Jun 21, 2019

Conversation

notmgsk
Copy link
Contributor

@notmgsk notmgsk commented Jun 16, 2019

The changes here are:

  • SBCL does not type-check a defclass's slots, unless you have (debug 3).
  • bt:destroy-thread does not appear to immediately destroy a thread. I think the behaviour of destroy-thread is implementation-dependent. To keep CCL happy, I explicitly loop until the thread is dead proper. If you don't do this, the threads hang around and you get very frustration and incomprehensible errors that you will spend you weekend trying to understand.

There are peculiarities of threading that I don't understand. I don't suppose this is the best solution, but it's the best I have for now.

Closes #72.

@notmgsk notmgsk requested a review from a team as a code owner June 16, 2019 23:46
Copy link
Contributor

@appleby appleby left a comment

Choose a reason for hiding this comment

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

Quick first round of comments. Probably take a more detailed look tomorrow.

src-tests/test-rpc.lisp Show resolved Hide resolved
src-tests/test-rpc.lisp Outdated Show resolved Hide resolved
src-tests/test-rpc.lisp Outdated Show resolved Hide resolved
src-tests/test-rpc.lisp Show resolved Hide resolved
src/rpcq.lisp Outdated Show resolved Hide resolved
src/rpcq.lisp Outdated Show resolved Hide resolved
Copy link
Contributor

@ecpeterson ecpeterson left a comment

Choose a reason for hiding this comment

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

This is fine, though it would make me more comfortable if there were a de/ser test that covered at least the vector case.

@ecpeterson
Copy link
Contributor

I just double-checked: there's a test-serialize-deserialize, but it's very light—too light to catch the changes we're making.

@notmgsk
Copy link
Contributor Author

notmgsk commented Jun 20, 2019

This is fine, though it would make me more comfortable if there were a de/ser test that covered at least the vector case.

OK. Hold off on merging, and I'll do that.

src-tests/suite.lisp Outdated Show resolved Hide resolved
src-tests/suite.lisp Outdated Show resolved Hide resolved
src-tests/test-rpc.lisp Show resolved Hide resolved
src-tests/test-rpc.lisp Show resolved Hide resolved
src-tests/test-rpc.lisp Show resolved Hide resolved
src-tests/test-rpc.lisp Show resolved Hide resolved
src/utilities.lisp Show resolved Hide resolved
@notmgsk notmgsk requested a review from stylewarning June 20, 2019 20:24
with-unique-rpc-address macro

for tests

declaim special *mocked-namespace*

silence warnings

if slee-p then sleep

Fix with-unique-rpc-address

Actually make the address unique

:list now maps to type vector (not simple-vector)

See #72 (comment)

Explicitly wait for server thread to terminate

It is my understanding that bt:destroy-thread does not make a
guarantee about *when* a thread will be destroyed, only that the
destroy signal has been delivered. Or something like that. This does
seem hacky.

declaim *mocked-namespace* in the correct location

Explicitly wait for thread to be destroyed in tests (fixes CCL)

On non-CCL use the previous method of just `(bt:destroy-thread
server-thread)`

redundant concatenate -> format

Comment grammar

export with-unique-rpc-address from rpcq package

Test serialize/deserialize messages with lists

Docstring for with-unique-rpc-address

Leave FIXME note regarding thread killers

array-total-size -> length

fix
@stylewarning stylewarning merged commit 512d53d into master Jun 21, 2019
@stylewarning stylewarning deleted the fix/ccl branch June 21, 2019 19:56
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.

Broken on CCL -- Slot RPCQ::|warnings| is unbound in #<RPCReply ...>
4 participants