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

Deadlock wollok #1689

Merged
merged 4 commits into from
Jul 18, 2019
Merged

Deadlock wollok #1689

merged 4 commits into from
Jul 18, 2019

Conversation

fdodino
Copy link
Collaborator

@fdodino fdodino commented Jul 12, 2019

@npasserini @PalumboN @lspigariol , no los quiero alarmar... pero le tengo bastante fe a estos toques. Estuve testeando con JConsole los deadlocks porque de casualidad me apareció y encontré el origen del problema:

la clase WollokGlobalScopeProvider tenía este método sincronizado

	override IScope getScope(IScope parent, Resource context, boolean ignoreCase, EClass type,
		Predicate<IEObjectDescription> filter) {
			var explicitImportedObjects = context.importedObjects
			if (filter !== null)
				explicitImportedObjects = explicitImportedObjects.filter(filter)
			val defaultScope = super.getScope(parent, context, ignoreCase, type, filter)
			new SimpleScope(defaultScope, explicitImportedObjects)
	}

El tema es que importedObjects también tenía su método sincronizado:

	def importedObjects(Resource context) {
		if (context === null || context.contents === null) {
			return #{}
		}
		val imports = context.calculateImports
		cache.get(context.URI, imports, [doImportedObjects(context, imports)])
	}

Fíjense que estamos accediendo a la cache que es MapBasedWollokGlobalScopeCache, y el método get, es (era) también sincronizado... y adentro todo bien si es sincronizado y la cache te trae los EObject de la URI. Pero si no está la URI la primera vez, ejecuta ifAbsentBlock.apply, que es el doImportedObjects de WollokGlobalScopeProvider.

	override get(URI uri, Set<String> imports, Function0<Iterable<IEObjectDescription>> ifAbsentBlock) {
		val uriString = uri.toString
		var cacheContent = cache.get(uriString) as MapBasedCacheContent
		if (cacheContent === null || cacheContent.mismatch(imports)) {
			cacheContent = new MapBasedCacheContent(uri, imports, ifAbsentBlock.apply)
			synchronized (this) {
				cache.put(uriString, cacheContent)
			}
		}
		cacheContent.result
	}

O sea, si estoy levantando el entorno como un campeón, y justo, justo, justo, el editor decide que quiere refrescar la vista, o bien cualquier otro elemento visual desde otro Thread necesita acceder a información de las URIs de Wollok, kaboom! En cuanto se entrelazan esos threads ninguno suelta al WollokGlobalScopeProvider y Wollok termina en un deadlock. Esto pasa especialmente cuando renombrás el archivo y el editor necesita refrescar los recursos asociados, y en algunos otros casos.

Lo ideal sería testearlo bastante, yo probé mucho agarrar un import y borrarlo y volverlo a poner, modificar el editor y tratar de grabarlo muchas veces rápido, para ver si no salta el popup molesto que aparecía hace algunos años y que se solucionó con estos synchronized. No me apareció ni una vez, y tampoco tuve deadlocks. Lo ideal sería que se bajen esta versión y lo prueben, sobre todo con workspaces rotos que no levantan ni a palos (a mí me anduvieron).

El único tema que sí noté es que cuando vos cambiás algún archivo de la biblioteca de Wollok (lang, lib, etc.) el workspace anterior no te sirve, queda roto (es como si quedara linkeado con los EObject viejos). Pero eso sigue siendo mejor que quedar bloqueado eternamente.

@fdodino fdodino added this to the Wollok v.1.8 Hipatia milestone Jul 12, 2019
@coveralls
Copy link

coveralls commented Jul 12, 2019

Coverage Status

Coverage remained the same at 89.173% when pulling cb5497d on deadlock-wollok into 2f7300e on dev.

@npasserini
Copy link
Member

npasserini commented Jul 12, 2019 via email

@fdodino
Copy link
Collaborator Author

fdodino commented Jul 18, 2019

Confirmado, todos estos días estuve abriendo nuevos branches, cada vez que tuve un deadlock cerré el runtime de Wollok, cambié las clases MapBasedWollokGlobalScopeCache y WollokGlobalScopeProvider (me se los cambios de memoria) y santo remedio, levantó el IDE perfectamente. Por otra parte no me tiró ninguna ventana de error el editor, así que yo diría con toda la fe de mergearlo y que lo empecemos a probar fuerte en nuestros entornos.

Copy link
Member

@npasserini npasserini left a comment

Choose a reason for hiding this comment

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

Dale masa!

@fdodino fdodino merged commit b3338dc into dev Jul 18, 2019
@fdodino fdodino deleted the deadlock-wollok branch July 18, 2019 03:01
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.

None yet

3 participants