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

Ensure instances of RsrConnection cannot be committed #142

Open
ericwinger opened this issue May 23, 2023 · 6 comments
Open

Ensure instances of RsrConnection cannot be committed #142

ericwinger opened this issue May 23, 2023 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@ericwinger
Copy link
Member

Every so often, not always, while replicating RSR services, I try to commit and get this error:

a TransactionError occurred (error 2407), The object Semaphore(396033793) may not be committed, 'cannot be committed, instances of its class are non-persistent'

After some finagling, I was able to list references and trace the semaphore back to the connection which every service holds onto (in memory because listReferences: and fastListReferences required aborts that would lose data)

Object _objectForOop: 396033793 "'Semaphore(396033793)'"
SystemRepository listReferencesInMemory: (Array with: (Object _objectForOop: 396033793))

Object _objectForOop: 395845121 "'aRsrThreadSafeNumericSpigot'"
SystemRepository listReferencesInMemory: (Array with: (Object _objectForOop: 395845121))

Object _objectForOop: 395700481 "'aRsrConnection'"
SystemRepository listReferencesInMemory: (Array with: (Object _objectForOop: 395700481))

Since I think services should be able to be persisted - even if just inadvertently - RSR shouldn't prevent that by holding onto instances of classes that are non-persistent objects.

@ericwinger ericwinger added the bug Something isn't working label May 23, 2023
@kurtkilpela
Copy link
Member

I believe, early on, it was decided that RsrService instances would not support commit once registered with a Session (Connection). This may be worth re-evaluating. We should consider setting options on the RsrService class to make that explicit.

@kurtkilpela
Copy link
Member

If possible, we may also consider allowing a Service to be committed but preventing the committing of the instance variables declared in RsrService.

@martinmcclure
Copy link
Member

My recollection is similar to Kurt's -- services are not supposed to be committed; to allow that wouldn't work very well, and would complicate the RSR implementation. Yes, setting the instancesNonPersistent option on RsrService would help catch this kind of problem earlier and make for an easier-to-understand error message.

@ericwinger
Copy link
Member Author

ericwinger commented May 23, 2023

I don't necessarily want to commit a service but I shouldn't be prevented from committing if a service gets stuck in a persistent object by mistake. That's an opportunity for a loss of work which is very frustrating and hard to debug.

@ericwinger
Copy link
Member Author

ericwinger commented May 25, 2023

I found the place in my code where some services were being put in UserGlobals. Method services retained their history beyond the life of the current session. While I can work around this need by not saving the service explicitly in UserGlobals (maybe save the class name, selector, source and meta only?) it still may be a good idea to allow services to persist. For example, if a developer is using the service as a model with persistent state as I was.

At minimum, if services are to remain non-persistable, it would be helpful if the RsrConnection instance was explicitly not persistable rather than just the Semaphore in the spigot. Easier to debug.

@kurtkilpela
Copy link
Member

After talking to Eric, it seems like he would be comfortable with us marking RsrConnection as non-persistent in GemStone. This would make it easier to debug.

I don't yet know if I can mark RsrConnection as non-persistent without having to move it into a GemStone-specific package. I'd really like to avoid doing that.

@kurtkilpela kurtkilpela changed the title don't hold onto instances of non-persistant classes Ensure instances of RsrConnection cannot be committed Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants