[incubator-toree] branch master updated: [TOREE-421] Allow thread groups (#178)

[lr...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

lresende pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-toree.git


The following commit(s) were added to refs/heads/master by this push:
     new 2443955  [TOREE-421] Allow thread groups (#178)
2443955 is described below

commit 2443955057be62e5c323a5d3473ea9a9fbdd3a08
Author: Kevin Bates <kb...@gmail.com>
AuthorDate: Fri Nov 1 12:13:59 2019 -0700

    [TOREE-421] Allow thread groups (#178)
    
    Remove the portions of Security Manager that restricted the
    creation of thread groups. This change also preserves the
    prevention of calls to System.exit() from sub-threads/groups
    (including those derived from the restricted group) - otherwise
    such calls from threads in a new thread group can terminate
    the kernel.
    
    I chose not to implement the ability to preserve the existing logic
    whereby the creation of new groups are prevented (i.e., via a new
    boolean option --prevent-new-thread-groups) since it felt like adding
    something that probably isn't necessary. If others disagree or we
    (eventually) find a good use-case for restoring that kind of functionality,
    we can add such an option at that time.
    
    Closes #178
---
 .../toree/security/KernelSecurityManager.scala     | 44 ++++++++--------------
 1 file changed, 15 insertions(+), 29 deletions(-)

diff --git a/kernel-api/src/main/scala/org/apache/toree/security/KernelSecurityManager.scala b/kernel-api/src/main/scala/org/apache/toree/security/KernelSecurityManager.scala
index fced5da..0b59554 100644
--- a/kernel-api/src/main/scala/org/apache/toree/security/KernelSecurityManager.scala
+++ b/kernel-api/src/main/scala/org/apache/toree/security/KernelSecurityManager.scala
@@ -40,9 +40,7 @@ object KernelSecurityManager {
    * Used to indicate which permissions to check. Only checks if the permission
    * is found in the keys and the value for that permission is true.
    */
-  private val permissionsToCheck: Map[String, Boolean] = HashMap(
-    "modifyThreadGroup" -> true
-  )
+  private val permissionsToCheck: Map[String, Boolean] = HashMap()
 
   /**
    * Checks whether the permission with the provided name is listed to be
@@ -105,42 +103,30 @@ class KernelSecurityManager extends SecurityManager {
       super.checkPermission(perm)
   }
 
-  override def getThreadGroup: ThreadGroup = {
-    val currentGroup = Thread.currentThread().getThreadGroup
-
-    // For restricted groups, we can only catch them in the checkAccess if we
-    // set the current group as the parent (to make sure all groups have a
-    // consistent name)
-    if (currentGroup.getName == RestrictedGroupName) {
-      new ThreadGroup(currentGroup, currentGroup.getName)
-    } else {
-      super.getThreadGroup
+  def _isRestrictedGroup(): Boolean = {
+    // Returns true if this thread group is derived from the restricted group so as to
+    // prevent System.exit(0) from sub-threads running in a different group.
+    var isRestricted = false
+    var currentGroup = Thread.currentThread().getThreadGroup
+
+    while ( currentGroup != null && !isRestricted ) {
+      if ( currentGroup.getName == RestrictedGroupName )
+        isRestricted = true
+      else
+        currentGroup = currentGroup.getParent
     }
-  }
-
-  override def checkAccess(g: ThreadGroup): Unit = {
-    //super.checkAccess(g)
-    if (g == null) return
-
-    val parentGroup = g.getParent
-
-    if (parentGroup != null &&
-      parentGroup.getName == RestrictedGroupName &&
-      g.getName != RestrictedGroupName)
-      throw new SecurityException("Not allowed to modify ThreadGroups!")
+    isRestricted
   }
 
   override def checkExit(status: Int): Unit = {
-    val currentGroup = Thread.currentThread().getThreadGroup
-
-    // Exit will be denied if this thread is in the restricted group AND the restricted
+    // Exit will be denied if this thread is derived from the restricted group AND the restricted
     // exit thread-local has not been enabled.  This will only happen when the request
     // came from jupyter via a (cell) execution_request indicating the cell is attempting
     // an exit call.  Proper notebook shutdown requests will go through the ShutdownHandler
     // where restricted exits are enabled and SIGINT (ctrl-c) requests are not performed
     // via the restricted group and thus allowed.
     //
-    if (currentGroup.getName == RestrictedGroupName ) {
+    if ( _isRestrictedGroup() ) {
       val isRestrictedExitEnabled:Option[Boolean] = Some(tlEnableRestrictedExit.get())
 
       if ( !isRestrictedExitEnabled.getOrElse(false) ) {