How to complete unload plugins PR? #1473

Closed
sonoro1234 opened this Issue May 18, 2015 · 11 comments

Comments

Projects
None yet
4 participants
@sonoro1234
Contributor

sonoro1234 commented May 18, 2015

commit 87e26cd
was merged before work was completed, so it introduced problems on scsynth quit. Now it must be solved someway

the work now has two possible PR

#1467
or
#1472

both of them solve program hangs and crash on win32, linux and macosx

the first one does plugin unload on scsynth and leave prepared for supernova
the second one reverts all the unloading plugins stuff and only does a minimal DiskIO_UGens modification

only one should be choosen and the opinion of everybody is important

Best
victor

@telephon

This comment has been minimized.

Show comment
Hide comment
@telephon

telephon May 22, 2015

Member

I'd like to contribute to the decision, but I can't judge the implications of the code – what is the main difference between the two?

Member

telephon commented May 22, 2015

I'd like to contribute to the decision, but I can't judge the implications of the code – what is the main difference between the two?

@sonoro1234

This comment has been minimized.

Show comment
Hide comment
@sonoro1234

sonoro1234 May 23, 2015

Contributor

Both of them solve program hangs and crash on win32, linux and macosx

The first one does plugin unload (calls unload from the plugin which is the oposite of load and can be used by the plugin to do deinitialization: deletes, thread:join, etc) on scsynth and leave it prepared for supernova (until novaserver destructor dont crash on macosx and linux)

The second one reverts all the unloading plugins stuff and only does a minimal DiskIO_UGens modification to avoid hang on exit

Contributor

sonoro1234 commented May 23, 2015

Both of them solve program hangs and crash on win32, linux and macosx

The first one does plugin unload (calls unload from the plugin which is the oposite of load and can be used by the plugin to do deinitialization: deletes, thread:join, etc) on scsynth and leave it prepared for supernova (until novaserver destructor dont crash on macosx and linux)

The second one reverts all the unloading plugins stuff and only does a minimal DiskIO_UGens modification to avoid hang on exit

@telephon

This comment has been minimized.

Show comment
Hide comment
@telephon

telephon May 23, 2015

Member

Ah ok, the question is whether to solve the problem generally (1) or just for the one case (2). Are there any implications of (2) that need to be taken into account (e.g. a special way you have to write the plugins?)

Member

telephon commented May 23, 2015

Ah ok, the question is whether to solve the problem generally (1) or just for the one case (2). Are there any implications of (2) that need to be taken into account (e.g. a special way you have to write the plugins?)

@sonoro1234

This comment has been minimized.

Show comment
Hide comment
@sonoro1234

sonoro1234 May 23, 2015

Contributor

In case 1 you can declare an unload function (if you need it to deinitialize your pluging) or dont declare unload and behaviour is as ever has been.
Case 2 is just a DiskIOUGens modification that solves the hang and nothing else

Contributor

sonoro1234 commented May 23, 2015

In case 1 you can declare an unload function (if you need it to deinitialize your pluging) or dont declare unload and behaviour is as ever has been.
Case 2 is just a DiskIOUGens modification that solves the hang and nothing else

@telephon

This comment has been minimized.

Show comment
Hide comment
@telephon

telephon May 23, 2015

Member

So it seems that (1) is the better solution, @timblechmann what do you think?

Member

telephon commented May 23, 2015

So it seems that (1) is the better solution, @timblechmann what do you think?

@timblechmann

This comment has been minimized.

Show comment
Hide comment
@timblechmann

timblechmann May 24, 2015

Contributor

(2) is keeps the leaks

Contributor

timblechmann commented May 24, 2015

(2) is keeps the leaks

@telephon

This comment has been minimized.

Show comment
Hide comment
@telephon

telephon May 24, 2015

Member

OK, I'll merge (2), yes?

Member

telephon commented May 24, 2015

OK, I'll merge (2), yes?

@timblechmann

This comment has been minimized.

Show comment
Hide comment
@timblechmann

timblechmann May 24, 2015

Contributor

please merge (1).

Contributor

timblechmann commented May 24, 2015

please merge (1).

@sonoro1234

This comment has been minimized.

Show comment
Hide comment
@sonoro1234

sonoro1234 May 25, 2015

Contributor

yes, merge (1) please.

Contributor

sonoro1234 commented May 25, 2015

yes, merge (1) please.

@telephon

This comment has been minimized.

Show comment
Hide comment
@telephon

telephon May 25, 2015

Member

OK :)

Member

telephon commented May 25, 2015

OK :)

@telephon

This comment has been minimized.

Show comment
Hide comment
@telephon

telephon May 25, 2015

Member

Done here: de18ba2

Member

telephon commented May 25, 2015

Done here: de18ba2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment