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

VM interface (API) is not synchonized #313

Open
layus opened this issue Aug 28, 2018 · 4 comments
Open

VM interface (API) is not synchonized #313

layus opened this issue Aug 28, 2018 · 4 comments
Assignees

Comments

@layus
Copy link
Member

layus commented Aug 28, 2018

There is an issue with the VM interface, or with the VM tests.
There is no synchronization in the VM implementation to ensure that the vm is really started after VM.start, nor that it is really stopped after VM.kill. The tests rely on this behavior, and fail randomly depending on scheduling.

Basically, all the calls to VM.list are unsafe, as the vm could finish before that call or linger from a previous test.

	new(proc {$}
	       Master={VM.current}
	       functor F
	       import
		  VM
	       define
		  {Send {VM.getPort Master} {VM.current}}
	       end
	       S={VM.getStream}
	       Other={VM.new F}
	    in
	       Other = 2
           %%% FAIL: VM could already be finished.
	       {VM.list} = [1 2]
	       S.1 = 2
	    end
	    keys:[mvm new stream])


	monitor(proc {$}
		   functor F
		   define
		      skip
		   end
		   Other={VM.new F}
		   S={VM.getStream}
		in
           %%% FAIL: VM could already be finished.
		   {VM.list} = [1 Other]
		   S.1 = terminated(Other reason:normal)


		   {VM.monitor Other} % already dead
		   S.2.1 = terminated(Other reason:unknown)


		   try
		      {VM.monitor {VM.current}}
		      fail
		   catch error(vm(cannotMonitorItself) ...) then
		      skip
		   end


		   try
		      {VM.monitor 12345}
		      fail
		   catch error(vm(invalidVMIdent) ...) then
		      skip
		   end
		end
		keys:[mvm new stream monitor])
@layus
Copy link
Member Author

layus commented Aug 28, 2018

Removing {VM.list} is not enough. The same issue arises with VM.getStream where the stream contains events from previous test. S.1 = terminated(Other reason:normal) fails because S.1 contains terminated(PreviousTestVM reason:normal) with PreviousTestVM \= Other because the old vm was still there during the creation of the new one.

These tests need a major rewrite. Disabling for now.

layus added a commit that referenced this issue Aug 28, 2018
@eregon eregon self-assigned this Aug 29, 2018
@eregon
Copy link
Member

eregon commented Aug 29, 2018

Thanks for the report.
Mmh, unfortunate, I thought I took care of making sure whether VMs survive tests wouldn't matter by making each test independent of the other.

Basically, all the calls to VM.list are unsafe

VM.list not unsafe, it's just inherently racy as of course a VM could terminate or start while the call returns. Just like any other way to observe the current state of a resource not controlled by the current thread.
Reading from a VM stream is an easy way to wait for a VM to start, and adding a monitor is a way to know and wait for a VM to end.
I'd guess the tests are to blame here, as you showed with inlined comments.

@layus It's been a while, how do you execute those tests?
Any tip to reproduce failures?

@layus
Copy link
Member Author

layus commented Aug 29, 2018

@eregon

VM.list not unsafe, it's just inherently racy as of course a VM could terminate or start while the call returns. Just like any other way to observe the current state of a resource not controlled by the current thread.

Exactly :-)

It's been a while, how do you execute those tests?

.../build/boosthost/emulator/ozemulator --home .../build .../build/platform-test/simple_runner.ozf .../build/platform-test/base/vm.ozf

You can also make install to get rid of the --home .../build args.

You will need make check beforehand to compile the test functors, but you can also compile them by hand.

make check runs the tests after compiling the dependencies; ctest runs the tests, but does not look at the dependencies; the above allows to run just one test.

Any tip to reproduce failures?

Run the tests in parallel to increase the load on the machine. I had good results with this one ;-)

seq 1 10000 | parallel -j 50 .../build/boosthost/emulator/ozemulator --home .../build .../build/platform-test/simple_runner.ozf .../build/platform-test/base/vm.ozf

@eregon
Copy link
Member

eregon commented Aug 29, 2018

I did the more basic thing of executing each test 10 times and found a bug (this first VM in the new test never terminates as there is no {VM.closeStream}).
I'll submit a PR when I have something stable :)

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

No branches or pull requests

2 participants